-
Notifications
You must be signed in to change notification settings - Fork 15
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
Remove the walrus operator for Python 3.6 compatibility #450
Conversation
Signed-off-by: Nikola Forró <[email protected]>
Build succeeded. ✔️ pre-commit SUCCESS in 1m 48s |
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.
dcermak can't run tests (and builds) internally - is there a way for a repo owner to overcome this?
I would say just by manual comment triggering
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.
it is just a test, so it is fine as it is.
But I am wondering why don't move the variables declarations at the beginning of the test, in an old style way, and use them later (instead of repeating the strings?)
Right, I actually did that on #445 but the test jobs kept erroring so I didn't even try on the other PR. But the incompatibility would have been caught already during build, so I should have done it anyway. |
You mean outside of test functions? I think that would make the readability worse. |
Build succeeded (gate pipeline). ✔️ pre-commit SUCCESS in 1m 47s |
6d1890a
into
main
we could also add the checks at least for builds to the required ones for merging |
Related to #444.
We still support EL8 and thus Python 3.6. CI didn't catch this because it didn't even run on RHEL8 (
dcermak can't run tests (and builds) internally
- is there a way for a repo owner to overcome this?).