scripts: compliance: add a check for missing west area maintainer enties #61499
Conversation
fabiobaltieri
requested review from
MaureenHelm,
carlescufi and
nashif
as code owners
zephyrbot
added
area: Process
area: Continuous Integration
area: Coding Guidelines
zephyrbot
requested review from
galak,
jfischer-no,
keith-zephyr and
stephanosio
fabiobaltieri
force-pushed
the
compliance-modules
branch
2 times, most recently
from
2397deb to
780f560
Compare
scripts/ci/check_compliance.py
Outdated
There was a problem hiding this comment.
Why recreate the manifest every time around the loop? This looks suspicious.
There was a problem hiding this comment.
I mean it's looping on two potential file names so it should only run once, but sure fair enough, reworked to code so it's not in a loop anymore.
fabiobaltieri
dismissed stale reviews from MaureenHelm and keith-zephyr
via
4f42a6a
fabiobaltieri
force-pushed
the
compliance-modules
branch
from
780f560 to
4f42a6a
Compare
There was a problem hiding this comment.
I admit I'm confused about what this is trying to do.
The MODULES_FILE variable is confusing me. That doesn't have to be the name of the main manifest file. west.yml is a convention; it's not enforced.
So then when you load the manifest using Manifest.from_file(), it could be opening a completely different file -- as determined by the manifest.path and manifest.file variables in the local workspace. There is no enforced connection with any west.yml file.
fabiobaltieri
force-pushed
the
compliance-modules
branch
from
4f42a6a to
ba5f1a0
Compare
My best interpretation of this is something like: "get the file name from Cool, done! |
|
@mbolivar-ampere ping |
There was a problem hiding this comment.
tested, does the job:
gale:zephyr(compliance-modules): ./scripts/ci/check_compliance.py
Running ModulesMaintainers tests in /home/nashif/zephyrproject/zephyr ...
Running KconfigBasicNoModules tests in /home/nashif/zephyrproject/zephyr ...
Running Kconfig tests in /home/nashif/zephyrproject/zephyr ...
Running MaintainersFormat tests in /home/nashif/zephyrproject/zephyr ...
Running ImageSize tests in /home/nashif/zephyrproject/zephyr ...
Running BinaryFiles tests in /home/nashif/zephyrproject/zephyr ...
Running Identity tests in /home/nashif/zephyrproject/zephyr ...
Running Pylint tests in /home/nashif/zephyrproject/zephyr ...
Running DevicetreeBindings tests in /home/nashif/zephyrproject/zephyr ...
Running Gitlint tests in /home/nashif/zephyrproject/zephyr ...
Running YAMLLint tests in /home/nashif/zephyrproject/zephyr ...
Running Checkpatch tests in /home/nashif/zephyrproject/zephyr ...
Running Nits tests in /home/nashif/zephyrproject/zephyr ...
Running GitDiffCheck tests in /home/nashif/zephyrproject/zephyr ...
Running KconfigBasic tests in /home/nashif/zephyrproject/zephyr ...
2 checks failed
ERROR : Test ModulesMaintainers failed:
Missing MAINTAINERS.yml entry for: "West project: lz5"
ERROR : Test Gitlint failed:
1: T8 Title is too short (2<5): "xx"
1: UC3 Commit title does not follow [subsystem]: [subject] (and should not start with literal subsys:): "xx"
1: UC6 Commit message body is empty, should at least have 1 line(s).
Complete results in compliance.xml
scripts/ci/check_compliance.py
Outdated
There was a problem hiding this comment.
This makes it look like the check could be run in any repository, not just the zephyr repository, but that doesn't quite make sense with what's going on below.
There was a problem hiding this comment.
Does it? Not sure it would make much sense as it is now but I certainly wouldn't prevent it, may need some extra check, guess it can be added if someone comes up with a use case for it.
fabiobaltieri
force-pushed
the
compliance-modules
branch
from
ba5f1a0 to
d035149
Compare
|
@MaureenHelm please revisit |
Add a check to ensure that every module has a corresponding maintainers file entry, ensure modules are not added with no recorded point of contact. Signed-off-by: Fabio Baltieri <[email protected]>
fabiobaltieri
dismissed stale reviews from mbolivar-ampere and keith-zephyr
via
0562f77
fabiobaltieri
force-pushed
the
compliance-modules
branch
from
d035149 to
0562f77
Compare
|
Rebased - had the first patch merged separately and the rebase wasn't skipping it correctly for some reason. Main patch is unchanged. |
6 participants
Add a compliance check to ensure all modules have a matching maintainer entry, add the two missing ones (added in #42580 and #56347)
@kristofer-jonsson-arm @najumon1980