-
Notifications
You must be signed in to change notification settings - Fork 755
Add # flake8: noqa directive to all failing files
#1585
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
e5990be to
65dc9e0
Compare
|
The author ping'd me about this so I've added it to the EUDevCall. I'm not really qualified to review the PR |
965282f to
0b66935
Compare
0b66935 to
4e2dda7
Compare
3a0220a to
598c66e
Compare
| submodules: 'recursive' | ||
|
|
||
| # Run flake8 but exclude third-party libraries | ||
| - run: pipx run flake8 --exclude=MAVProxy/modules/mavproxy_cesium,MAVProxy/modules/lib |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use --show-source in run_flake8 - any reason not to include it here?
Note that cesium is external, but most, if not all, of MAVProxy/modules/lib is ours to clean. Why did you choose to exclude it entirely here rather than add the # flake8: noqa?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, let's add --show-source. (Ruff has no such option, so I have gotten out of the practice of adding it.)
https://github.com/ArduPilot/MAVProxy/tree/master/MAVProxy/modules says:
mavproxy_cesium @ 78f7be3
Therefore, the code in MAVProxy/modules/mavproxy_cesium is a git submodule and is not in this repository.
| run: | | ||
| python -m pip install -U flake8 | ||
| run: python -m pip install --upgrade flake8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revert these patches. They are unrelated to the topic of the PR and don't improve things.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The switch from -U to --upgrade is a self-documenting readability change. As I have stated in other ArduPilot PRs, pip install currently has six different --u* command-line options, including the often-used --user.
% python3 -m pip install --help | grep "\-\-u"
598c66e to
0e36f77
Compare
|
On Thu, 21 Aug 2025, Christian Clauss wrote:
Therefore, the code in MAVProxy/modules/mavproxy_cesium is a git submodule and is not in this repository.
Correct, and we shouldn't check it. But the other directory being
excluded is not a submodule - and it even has a couple of flake8-clean
files...
|
0e36f77 to
18ab3b0
Compare
18ab3b0 to
3a2b8aa
Compare
Run flake8 in GitHub Actions but exclude third-party libraries
run: pipx run flake8 --exclude=MAVProxy/modules/libOnce this has been running for a while, we can:
scripts/run_flake8.pygit grep AP_FLAKE8_CLEAN)ArduPilot/ardupilot#30288 (comment)
%
flake8 --exclude=MAVProxy/modules/lib --disable-noqa --count