We read every piece of feedback, and take your input very seriously.
To see all available qualifiers, see our documentation.
There was an error while loading. Please reload this page.
end-of-file-fixer
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
let's formalize suggestion from #1888 (comment)
Sorry, something went wrong.
precommit: enable end-of-file-fixer
9e67138
apply
96e21f0
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Although I think the underlying concept here is good, either all or all but one of the changes inside test/fixtures should be omitted.
test/fixtures
Most files in test/fixtures are test data. Test data should rarely if ever modified for stylistic reasons, because it can break tests, including in ways whose impact would only occur in the case of a regression that a test would otherwise catch. In other words, changing test data without fully and carefully examining exactly how that change might relate to each use of the data in the tests can cause tests to break in ways that running the tests does not reveal.
The changes here do break the tests. Fortunately, they break them in a way that is easy to catch: a test fails that is supposed to pass. They may or may not also break the tests in other ways that are not easy to catch, i.e., that don't affect their status now, but that make the tests pass in slightly more circumstances than they should, thereby weakening them against future inadvertent bugs.
In the case of test/fixtures/.gitconfig, it may be worthwhile to look into this and maybe make the change anyway, since that file is not just used as a fixture: it is also used to configure CI. In the case of all other files in the test/fixtures directory that are modified here, I do not think there is sufficient benefit to justify the added time and effort that it would take to achieve a reasonable level of confidence that the changes would not cause any tests to continue passing in any circumstances under which they should start failing.
test/fixtures/.gitconfig
Although no *.py files inside test/fixtures are changed here, those files, while they are considered fixtures, are not test data, so I think it is fine to perform these kinds of stylistic checks and fixes on them. Other files in test/fixtures, with the possible exception of test/fixtures/.gitconfig, really should not be subject to these kinds of checks or receive these kinds of changes.
*.py
See also #1888 (comment).
The changes outside of test/fixtures, including the automated changes in doc/source, all look good to me.
doc/source
validate-pyproject
Thanks for the initiative!
I am now officially setting this PR to a state that indicates that some modifications are needed.
exclude: test/fixtures/
366a607
reverted the changes in tests/fixtures and added this folder to be ignored
tests/fixtures
LGTM
@Byron mind have a look again pls
Thanks a lot!
3d3c86c
Byron Byron approved these changes
EliahKagan EliahKagan approved these changes
Successfully merging this pull request may close these issues.