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

precommit: enable validate-pyproject #1921

Merged
merged 2 commits into from
May 31, 2024

Conversation

Borda
Copy link
Contributor

@Borda Borda commented May 30, 2024

let's formalize suggestion from #1888 (comment)

@Borda Borda mentioned this pull request May 30, 2024
Copy link
Contributor

@EliahKagan EliahKagan left a comment

Choose a reason for hiding this comment

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

Thanks--this is a good idea!

This edits .pre-commit-config.yaml in such a way that the file no longer has a trailing newline character. Although we have a pre-commit hook that scans YAML, it apparently doesn't check for that. If the changes from #1920 are merged--even with my suggested modifications, since this file is outside of test/fixtures--then they should catch it (though if you are aware of more comprehensive YAML linters that should be used instead of, or in addition, to the current one, then I think that could be a valuable, though separate, improvement).

Leaving out the newline here is minor, and there is nothing else that should change here. So it should actually be okay to merge this as it stands. However, some ways of integrating both this and #1920 could then result in a lint failure on the main branch even if both PR branches are green when merged. (Also, regardless of what happens with #1920, it is slightly better here to have the trailing newline.) So I think it would be even better to add the trailing newline here.

.pre-commit-config.yaml Outdated Show resolved Hide resolved
Just to be sure the coming linting won't be disturbed by that.
Copy link
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

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

Thanks a lot for your help!

@Byron Byron merged commit 1e1a858 into gitpython-developers:main May 31, 2024
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants