Skip to content
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

More concise 'fail-under' >= comparisons #9514

Closed

Conversation

jtkmckenna
Copy link

Type of Changes

Type
βœ“ πŸ› Bug fix

Description

Change the handling of 'fail-under' from a float to a string. When fail-under's comparison is made, we can round the score to the same precision as the fail-under.

Example, if a project reports a score of 8.79, to fix all pylint warnings is a large task, so we add a CI task that ensures the score only increases on each pull request.

If I use:

pylint iterate --fail-under 8.79

The CI task will run and return a non-zero exit code. This is because the score is more precisely '8.78563061304178', and the comparison

score_value >= linter.config.fail_under

is False. The documentation accurately describes the >= operator, but users can be left confused why the --fail-under only behaves some of the time (ie when the score rounds down in the reported score).
The help string for --fail-under:

Specify a score threshold under which the program will exit with error. (default: 10)

Here in this pull request, I set a local variable with a score rounded to the same number of decimal places as the user provided. If the user provides an integer (or if the default value is set), I do not round the score

Round score to match precision of user defined limit
@Pierre-Sassoulas
Copy link
Member

Thank you for opening a merge request and documenting your proposed changes. I think keeping a float and failing a score of 8.789 when the fail under option is 8.79 is what is expected. If I used --fail-under 9 and the score was 8.6, I would expect pylint to fail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants