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

[#149] Make check and diff not mutually exclusive #155

Closed
wants to merge 6 commits into from
Closed

[#149] Make check and diff not mutually exclusive #155

wants to merge 6 commits into from

Conversation

csurfer
Copy link
Contributor

@csurfer csurfer commented Apr 21, 2018

This change adds the ability to use both --check and --diff together to see the proposed changes and adds a test to verify that these options can indeed be called together.

@coveralls
Copy link

coveralls commented Apr 21, 2018

Pull Request Test Coverage Report for Build 266

  • 47 of 49 (95.92%) changed or added relevant lines in 2 files are covered.
  • 65 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.3%) to 95.147%

Changes Missing Coverage Covered Lines Changed/Added Lines %
black.py 37 39 94.87%
Files with Coverage Reduction New Missed Lines %
black.py 65 94.25%
Totals Coverage Status
Change from base Build 261: 0.3%
Covered Lines: 1745
Relevant Lines: 1834

💛 - Coveralls

@@ -595,6 +595,15 @@ def test_write_cache_write_fail(self) -> None:
mock.side_effect = OSError
black.write_cache({}, [])

def test_check_diff_use_together(self) -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of creating a temp file (which is slow), just run -the command on two existing files. I suggesttests/string_quotes.py and tests/composition.py. The former should return exit code 1, the latter should return exit code 0. The former should produce a diff, the latter should not produce a diff.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do. Also realized a mistake on my part. Report can no longer be initiated with check=write_back is WriteBack.NO. Correcting that too and adding tests as suggested.

@ambv
Copy link
Collaborator

ambv commented Apr 21, 2018

Thanks for your pull request!

@csurfer
Copy link
Contributor Author

csurfer commented Apr 22, 2018

@ambv : Looks like an issue with github. Not reordering the commits after the change in the fork. Let me abandon this pull request and raise another for ease of review.

@csurfer csurfer closed this Apr 22, 2018
@csurfer
Copy link
Contributor Author

csurfer commented Apr 22, 2018

No luck. Let me know if I need to do something to make it easier.

@csurfer csurfer reopened this Apr 22, 2018
@ambv
Copy link
Collaborator

ambv commented Apr 23, 2018

Oh man, this PR now includes way more changes than you intended. You will probably want to create a new pull request, which will probably be faster for you than recovering from this mess. This time create a branch for your changes which will make rebasing easier and less error-prone.

To not lose your changes, pipe git show YOUR_REVISION >patch.diff of your revision to a file. Then on a clean branch you can reapply the diff with patch -p1 <patch.diff.

@csurfer csurfer closed this Apr 23, 2018
@CMCDragonkai
Copy link

Is there a MR for this? I'd like to use --check and --diff together in CI to show the team what they need to change to pass MR review?

@hugovk
Copy link
Contributor

hugovk commented Sep 19, 2019

They're already available in the released version.

https://github.com/psf/black/blob/master/README.md#installation-and-usage

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.

7 participants