-
Notifications
You must be signed in to change notification settings - Fork 215
Enforce Python Code Style according to CODEOWNERS #10330
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
base: master
Are you sure you want to change the base?
Conversation
👋 Hello! Thanks for contributing to our project. You can see the progress at the end of this page and at https://github.com/uyuni-project/uyuni/pull/10330/checks If you are unsure the failing tests are related to your code, you can check the "reference jobs". These are jobs that run on a scheduled time with code from master. If they fail for the same reason as your build, it means the tests or the infrastructure are broken. If they do not fail, but yours do, it means it is related to your code. Reference tests: KNOWN ISSUES Sometimes the build can fail when pulling new jar files from download.opensuse.org . This is a known limitation. Given this happens rarely, when it does, all you need to do is rerun the test. Sorry for the inconvenience. For more tips on troubleshooting, see the troubleshooting guide. Happy hacking! |
There don't work and haven't been touched in a long time. If anyone want to resurrect them, they're still in the git history.
Found by Sonarqube's python:S5361 https://sonarcloud.io/organizations/uyuni-project/rules?open=python%3AS5361&rule_key=python%3AS5361 Using re.sub() where str.replace() works is less performant, the regex engine should only be used when it's required.
Anyone who's got pulled in by being a code owner (apart from the python group), if you're not okay with the changes, please let me know and I'll revert them: We'd like to have all python code following our style guide, but the respective code owners can decide for themselves. Executable Python programs are currently not tracked in the CODEOWNERS definition, this is where some of you got pulled in. @cbbayburt I put the changelog checker changes in a separate commit since I remember you saying you weren't sure about our style. Just let me know if I should drop it. |
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.
Hi @agraul, I think we agreed to don't include testsuite folder in those changes 🤔
Anyway, if you already made it, fine to keep it. It was more about reducing your workload.
Yes, you're right. Sorry for that, I excluded them from the pylint stuff but let |
It's fine, I doubt the re-format will break anything. Let's keep it. |
Thanks, @agraul for remembering that :) |
What does this PR change?
This pull request enforces the Google Python Style for all Python files owned by @uyuni-project/python. With the current CODEOWNERS setup, that's close to
**/*.py
.As with previous, pull requests,
pylint: disable=$symbol
comments are added to allow what's currently broken. Only new style guide violations will be caught by the Github check.Additionally, new Python files will be checked by the Github check by default. To opt-out of that, please adjust both
CODEOWNERS
and the "Python Checkstyle" workflow configuration. Existing exclusions show how its done.Moreover, leftover Python 2 code is removed in this PR.
GUI diff
No difference.
Documentation
No documentation needed: only internal and user invisible changes
DONE
Test coverage
No semantic difference in source code.
Links
Issue(s): https://github.com/SUSE/spacewalk/issues/26607
Port(s): # add downstream PR(s), if any
Changelogs
Make sure the changelogs entries you are adding are compliant with https://github.com/uyuni-project/uyuni/wiki/Contributing#changelogs and https://github.com/uyuni-project/uyuni/wiki/Contributing#uyuni-projectuyuni-repository
If you don't need a changelog check, please mark this checkbox:
If you uncheck the checkbox after the PR is created, you will need to re-run
changelog_test
(see below)Re-run a test
If you need to re-run a test, please mark the related checkbox, it will be unchecked automatically once it has re-run:
Before you merge
Check How to branch and merge properly!