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
Add min_pyver_end_position
option
#5386
Conversation
c6cce06
to
3615219
Compare
Pull Request Test Coverage Report for Build 1502692283
💛 - Coveralls |
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.
Look good already, I have some minor remarks.
@@ -72,6 +72,15 @@ You can also use ``# +n: [`` with n an integer if the above syntax would make th | |||
|
|||
If you need special control over Pylint's configuration, you can also create a .rc file, which | |||
can have sections of Pylint's configuration. | |||
The .rc file can also contain a section ``[testoptions]`` to pass options for the functional |
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.
👌 This is very useful ! I wish I had that when starting out.
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.
I didn't had much time the last few days. Just trying to catch up now.
min_pyver_end_position
was exactly what I though off with my proposal. At the moment I agree that we don't need it. However there are still some AST node types left that don't have position information and just might get some in a future release.
"min_pyver": Minimal python version required to run the test | ||
"max_pyver": Maximum python version required to run the test |
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.
This isn't completely true. Whereas min_pyver
acts inclusively, max_pyver
is exclusive due to the way tuple comparisons and sys.version_info
work in Python.
I.e. an alpha release would be (3, 11, 0, 'alpha', 0) > (3, 11)
. Thus setting max_pyver = 3.11
effectively excludes 3.11
and above.
IMO that is the desired behavior as otherwise one would need to say something like 3.10.99
to exclude 3.11+
. So just the documentation that should get updated.
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.
Thanks @cdce8p for reviewing even after we already merged, much appreciated!
I'll create a PR that updates the doc.
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.
@cdce8p Any good ideas about how to describe this. I have been trying for 5 minutes but can't come up with a simple but correct sentence 😆
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.
"max_pyver": Starting from that python version for the test won't be run
?
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.
See #5411
doc/whatsnew/<current release.rst>
.Type of Changes
Description
Ref #5349 (comment).