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

Validate PEP created dates and update linters #1886

Merged
merged 3 commits into from
Mar 22, 2021

Conversation

CAM-Gerlach
Copy link
Member

Followup/replacement to @hugovk 's PR #1809 .

Instead of adding a bespoke script to validate created dates, which runs into locale and cross-platform issues and has non-trivial complexity and performance costs, this instead simply adds a pygrep hook which performs equivalent validation, checking that all PEPs have a created date, and that the date validates to the current bespoke format. This caught two new instances of created dates in the standard ISO 8601/RFC 3339 format, which this PR changes to the bespoke one used on this repo.

Additionally, PEP 553 failed the validation because it has mixed line endings; unlike every other file in the repo that is pure \n/LF (aside from the .gitignore), it contains \r\n line breaks. To resolve this, it is normalized to \n, and to avoid this in the future without the need for any additional user action, .gitattributes is set so that Git will automatically normalize line breaks on checkin, and a pre-commit built-in hook is used to validate that locally and on the CIs.

Finally, I bumped the pygrep hooks version and adjusted the files/types params to properly check both txt and rst PEPs following pre-commit/pygrep-hooks#51 . In doing so, the rst-directive-colons hook caught (and I fixed) an issue with PEP 540, where a note directive didn't have the correct number of colons after it, making it into a rst comment and caused the information it contained to disappear completely from the rendered PEP.

Its easy and cheap to add additional sub-hooks to validate other PEP fields against regexes, either their presence, content or both (e.g. author, PEP number, content type, status, etc), so if that would be useful, just let me know.

To keep the scope of this PR constrained, I didn't apply the fix to the rst-backticks hook so it would check the .txt-extension PEPs, as it discovered over 300 instances (apparently all true positives) of erroneous single backticks, most around inline code, filenames or other elements intended to be verbatim, others mismatched, and in a number of malformed links. I can address that in a separate PR along the lines of #1797 / #1802 .

Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

Nice, thanks! This all makes sense to me.

Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@CAM-Gerlach
Copy link
Member Author

Thanks for your work making this possible @hugovk !

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Okay, I'll land it. Thanks!

@gvanrossum gvanrossum merged commit e6fb0d8 into python:master Mar 22, 2021
@CAM-Gerlach
Copy link
Member Author

Thanks @gvanrossum !

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.

5 participants