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

Soft comparison of --required-version #2832

Merged
merged 10 commits into from Jan 30, 2022
Merged

Soft comparison of --required-version #2832

merged 10 commits into from Jan 30, 2022

Conversation

freud14
Copy link
Contributor

@freud14 freud14 commented Jan 30, 2022

Description

Add change suggested in #2831.

Checklist - did you ...

  • Add a CHANGELOG entry if necessary?
  • Add / update tests if necessary?
  • Add new / update outdated documentation?

Copy link
Collaborator

@JelleZijlstra JelleZijlstra left a comment

Thanks! This behavior should also be documented in the @click.option help string for --required-version, which gets mirrored in https://black.readthedocs.io/en/stable/usage_and_configuration/the_basics.html#command-line-options.

CHANGES.md Outdated Show resolved Hide resolved
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
src/black/__init__.py Outdated Show resolved Hide resolved
@freud14
Copy link
Contributor Author

@freud14 freud14 commented Jan 30, 2022

Alright, now it should be ok. There is only test_skip_magic_trailing_comma that doesn't pass on my machine but I suspect it is a matter of configuration.

Copy link
Collaborator

@felix-hilden felix-hilden left a comment

Thanks for submitting! I'd also want to consider the option of not allowing for detailed control at all. Meaning we'd only pass an integer for the major version. But if we decide to allow the detailed version, I'd like to see a test where the major is correct but the minor is different, which currently I'd expect to fail.

@jack1142
Copy link
Contributor

@jack1142 jack1142 commented Jan 30, 2022

I think that both exact and major-only matching should be supported as I would say it's quite common to pin formatters and linters to exact versions to ensure the repeatable experience (even if you claim that there won't be any changes to Black's formatting between versions).

@github-actions
Copy link

@github-actions github-actions bot commented Jan 30, 2022

diff-shades reports zero changes comparing this PR (0ecaaf2) to main (f61299a).


What is this? | Workflow run | diff-shades documentation

@freud14
Copy link
Contributor Author

@freud14 freud14 commented Jan 30, 2022

@felix-hilden I'm not sure what you meant but I just added a new test. Let me know if it's what you meant.

@freud14
Copy link
Contributor Author

@freud14 freud14 commented Jan 30, 2022

For the debate about keeping the detailed version, I personally don't care very much. If you tell me you don't want to keep the detailed version, I will make the changes.

@JelleZijlstra
Copy link
Collaborator

@JelleZijlstra JelleZijlstra commented Jan 30, 2022

Let's keep the detailed version too.

Copy link
Collaborator

@felix-hilden felix-hilden left a comment

I think allowing for the full version is okay 👍

src/black/__init__.py Outdated Show resolved Hide resolved
Co-authored-by: Felix Hildén <felix.hilden@gmail.com>
Copy link
Collaborator

@felix-hilden felix-hilden left a comment

Looks good now! Final nit below. Thanks a bunch for the quick updates!

tests/test_black.py Outdated Show resolved Hide resolved
Co-authored-by: Felix Hildén <felix.hilden@gmail.com>
@JelleZijlstra JelleZijlstra merged commit cae7ae3 into psf:main Jan 30, 2022
38 checks passed
@freud14 freud14 deleted the patch-1 branch Jan 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants