-
Notifications
You must be signed in to change notification settings - Fork 527
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
feat: Add pre-commit config #1017
base: dev
Are you sure you want to change the base?
feat: Add pre-commit config #1017
Conversation
* Add pre-commit/pre-commit-hooks for general linting. - Ignore CSV files. * Add python-jsonschema/check-jsonschema for GitHub Actions. schema validation * Add pre-commit.ci config. - Set autoupdate schedule to quarterly (the longest possible).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would also benefit from codespell-project/codespell being added (c.f. https://github.com/scipy-conference/scipy-conference/blob/3755b74eb94a00ca7038b5210f01f021b5127e72/.pre-commit-config.yaml#L30-L38) but given that there are multiple words that need to get added to an ignore list and fixed in files, I've decided to not do that in this PR. as it would need some input from other parties.
@scipy-conference/proceedings this is ready for review. Let me know if you have any questions. @tkoyama010 a review from you would be welcome also. :) |
A markdown formatter should also be added, but the impact is too great and should be discussed carefully. |
Co-authored-by: Tetsuo Koyama <[email protected]>
* Also fix the following errors: publisher/_static/scipy-proc.css [error] publisher/_static/scipy-proc.css: SyntaxError: CssSyntaxError: Missed semicolon (28:23) [error] 26 | .abstract { [error] 27 | width: 80ex; [error] > 28 | text-align: justify [error] | ^ [error] 29 | padding-left: 1em; [error] 30 | } [error] 31 | publisher/_static/screen.css [error] publisher/_static/screen.css: SyntaxError: CssSyntaxError: Unknown word (57:3) [error] 55 | float: right; [error] 56 | margin: 0.5ex 10px 0.5ex 0px; [error] > 57 | padding 0; [error] | ^ [error] 58 | display: inline; /* forces IE to calc correct margin */ [error] 59 | } [error] 60 |
I like the idea, but as it would be a big diff Results of applying igorshubovych/markdownlint-cli:markdownlint.............................................................Failed
- hook id: markdownlint
- exit code: 1
publisher/getting_ready_for_new_year.md:8:1 MD051/link-fragments Link fragments should be valid [Expected: #what-problem-are-we-solving; Actual: #What-problem-are-we-solving] [Context: "[the problem section](#What-problem-are-we-solving)"]
publisher/getting_ready_for_new_year.md:10 MD025/single-title/single-h1 Multiple top-level headings in the same document [Context: "# Workflow"]
publisher/getting_ready_for_new_year.md:30:81 MD013/line-length Line length [Expected: 80; Actual: 99]
publisher/getting_ready_for_new_year.md:32:81 MD013/line-length Line length [Expected: 80; Actual: 88]
publisher/getting_ready_for_new_year.md:33:81 MD013/line-length Line length [Expected: 80; Actual: 91]
publisher/getting_ready_for_new_year.md:74:49 MD026/no-trailing-punctuation Trailing punctuation in heading [Punctuation: '.']
publisher/getting_ready_for_new_year.md:89:65 MD026/no-trailing-punctuation Trailing punctuation in heading [Punctuation: '.']
publisher/getting_ready_for_new_year.md:94:1 MD051/link-fragments Link fragments should be valid [Expected: #workflow; Actual: #Workflow] [Context: "[](#Workflow)"]
publisher/README.md:1:41 MD026/no-trailing-punctuation Trailing punctuation in heading [Punctuation: '.']
publisher/README.md:5:81 MD013/line-length Line length [Expected: 80; Actual: 153]
publisher/README.md:7:81 MD013/line-length Line length [Expected: 80; Actual: 91]
publisher/README.md:9:81 MD013/line-length Line length [Expected: 80; Actual: 132]
publisher/README.md:11:17 MD026/no-trailing-punctuation Trailing punctuation in heading [Punctuation: ':']
publisher/README.md:23:81 MD013/line-length Line length [Expected: 80; Actual: 91]
publisher/README.md:27:81 MD013/line-length Line length [Expected: 80; Actual: 242]
publisher/README.md:31:81 MD013/line-length Line length [Expected: 80; Actual: 125]
publisher/README.md:47:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
publisher/README.md:48:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
publisher/README.md:50:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
publisher/README.md:51:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
publisher/README.md:57:81 MD013/line-length Line length [Expected: 80; Actual: 148]
publisher/README.md:67:81 MD013/line-length Line length [Expected: 80; Actual: 93]
publisher/README.md:83:81 MD013/line-length Line length [Expected: 80; Actual: 91]
publisher/README.md:109:1 MD023/heading-start-left Headings must start at the beginning of the line [Context: " ## DOI metadata"]
publisher/README.md:116:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
publisher/README.md:117:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
publisher/README.md:118:1 MD007/ul-indent Unordered list indentation [Expected: 4; Actual: 8]
publisher/README.md:119:1 MD007/ul-indent Unordered list indentation [Expected: 4; Actual: 8]
publisher/README.md:120:1 MD007/ul-indent Unordered list indentation [Expected: 4; Actual: 8]
publisher/README.md:124:2 MD034/no-bare-urls Bare URL used [Context: "http://www.issn.org/understand..."]
publisher/README.md:136:2 MD034/no-bare-urls Bare URL used [Context: "https://doi.org/10.25080/shinm..."]
publisher/README.md:137 MD032/blanks-around-lists Lists should be surrounded by blank lines [Context: "- which resolves the proceedin..."]
pull_request_template.md:1 MD041/first-line-heading/first-line-h1 First line in a file should be a top-level heading [Context: "If you are creating this PR in..."]
pull_request_template.md:2:5 MD034/no-bare-urls Bare URL used [Context: "http://procbuild.scipy.org/"]
pull_request_template.md:8 MD012/no-multiple-blanks Multiple consecutive blank lines [Expected: 1; Actual: 2]
README.md:9:81 MD013/line-length Line length [Expected: 80; Actual: 116]
README.md:12:81 MD013/line-length Line length [Expected: 80; Actual: 129]
README.md:17:81 MD013/line-length Line length [Expected: 80; Actual: 133]
README.md:42 MD032/blanks-around-lists Lists should be surrounded by blank lines [Context: "- with many people contributin..."]
README.md:43:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
README.md:44:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
README.md:46:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
README.md:46:21 MD034/no-bare-urls Bare URL used [Context: "https://github.com/scipy-confe..."]
README.md:47:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
README.md:47:15 MD034/no-bare-urls Bare URL used [Context: "https://github.com/scipy-confe..."]
README.md:50 MD032/blanks-around-lists Lists should be surrounded by blank lines [Context: "- build system:"]
README.md:51:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
README.md:52:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
README.md:53:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
README.md:55:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
README.md:56:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
README.md:57:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
README.md:58:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
README.md:73:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
README.md:74:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
README.md:74:81 MD013/line-length Line length [Expected: 80; Actual: 96]
README.md:82:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
README.md:83:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
README.md:86:34 MD034/no-bare-urls Bare URL used [Context: "http://conference.scipy.org/pr..."]
README.md:103 MD012/no-multiple-blanks Multiple consecutive blank lines [Expected: 1; Actual: 2]
README.md:106:81 MD013/line-length Line length [Expected: 80; Actual: 164]
README.md:109 MD032/blanks-around-lists Lists should be surrounded by blank lines [Context: "- Meghann Agarwal (@mepa)"]
README.md:119:81 MD013/line-length Line length [Expected: 80; Actual: 115]
README.md:138:81 MD013/line-length Line length [Expected: 80; Actual: 271]
README.md:169:51 MD026/no-trailing-punctuation Trailing punctuation in heading [Punctuation: ':']
README.md:173:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
README.md:174:1 MD007/ul-indent Unordered list indentation [Expected: 4; Actual: 8]
README.md:175:1 MD007/ul-indent Unordered list indentation [Expected: 4; Actual: 8]
README.md:176:1 MD007/ul-indent Unordered list indentation [Expected: 4; Actual: 8]
README.md:177:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
README.md:178:53 MD034/no-bare-urls Bare URL used [Context: "http://procbuild.scipy.org"]
README.md:182:81 MD013/line-length Line length [Expected: 80; Actual: 112]
README.md:183:81 MD013/line-length Line length [Expected: 80; Actual: 126]
README.md:190:81 MD013/line-length Line length [Expected: 80; Actual: 135]
README.md:210 MD040/fenced-code-language Fenced code blocks should have a language specified [Context: "```"]
README.md:216 MD040/fenced-code-language Fenced code blocks should have a language specified [Context: "```"]
README.md:224:81 MD013/line-length Line length [Expected: 80; Actual: 99]
README.md:227:81 MD013/line-length Line length [Expected: 80; Actual: 123]
README.md:228:81 MD013/line-length Line length [Expected: 80; Actual: 167]
README.md:228:141 MD034/no-bare-urls Bare URL used [Context: "http://procbuild.scipy.org"]
README.md:228:100 MD051/link-fragments Link fragments should be valid [Context: "[check your paper](#check-your-paper)"]
README.md:228:41 MD051/link-fragments Link fragments should be valid [Context: "[push changes to your PR's branch](#push-your-changes)"]
README.md:241:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
README.md:242:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
README.md:244:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
README.md:246:12 MD038/no-space-in-code Spaces inside code span elements [Context: "`git remote -v `"]
README.md:247 MD031/blanks-around-fences Fenced code blocks should be surrounded by blank lines [Context: "```"]
README.md:247 MD040/fenced-code-language Fenced code blocks should have a language specified [Context: "```"]
README.md:248:7 MD010/no-hard-tabs Hard tabs [Column: 7]
README.md:249:7 MD010/no-hard-tabs Hard tabs [Column: 7]
README.md:250:9 MD010/no-hard-tabs Hard tabs [Column: 9]
README.md:251:9 MD010/no-hard-tabs Hard tabs [Column: 9]
README.md:257:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
README.md:259:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
README.md:268 MD040/fenced-code-language Fenced code blocks should have a language specified [Context: "```"]
README.md:276:81 MD013/line-length Line length [Expected: 80; Actual: 96]
README.md:280:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
README.md:281:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
README.md:287:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
README.md:311:81 MD013/line-length Line length [Expected: 80; Actual: 116]
README.md:345:81 MD013/line-length Line length [Expected: 80; Actual: 174]
README.md:347:81 MD013/line-length Line length [Expected: 80; Actual: 96]
README.md:381:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
README.md:384:81 MD013/line-length Line length [Expected: 80; Actual: 85]
README.md:390:81 MD013/line-length Line length [Expected: 80; Actual: 154]
README.md:399 MD012/no-multiple-blanks Multiple consecutive blank lines [Expected: 1; Actual: 2]
README.md:402:1 MD007/ul-indent Unordered list indentation [Expected: 0; Actual: 1]
README.md:403:1 MD007/ul-indent Unordered list indentation [Expected: 0; Actual: 1]
README.md:406:1 MD007/ul-indent Unordered list indentation [Expected: 0; Actual: 1]
README.md:407:1 MD007/ul-indent Unordered list indentation [Expected: 0; Actual: 1]
README.md:410:30 MD026/no-trailing-punctuation Trailing punctuation in heading [Punctuation: ':']
README.md:412 MD040/fenced-code-language Fenced code blocks should have a language specified [Context: "```"]
README.md:424 MD040/fenced-code-language Fenced code blocks should have a language specified [Context: "```"]
README.md:425:81 MD013/line-length Line length [Expected: 80; Actual: 248]
README.md:430:62 MD034/no-bare-urls Bare URL used [Context: "http://procbuild.scipy.org"]
README.md:448:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
README.md:448:81 MD013/line-length Line length [Expected: 80; Actual: 95]
README.md:455:1 MD007/ul-indent Unordered list indentation [Expected: 0; Actual: 2]
README.md:455 MD032/blanks-around-lists Lists should be surrounded by blank lines [Context: "- The **labels** in question a..."]
README.md:456:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
README.md:457:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
README.md:458:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
README.md:460:81 MD013/line-length Line length [Expected: 80; Actual: 134]
README.md:470:1 MD007/ul-indent Unordered list indentation [Expected: 2; Actual: 4]
README.md:475 MD001/heading-increment Heading levels should only increment by one level at a time [Expected: h3; Actual: h4]
README.md:479:81 MD013/line-length Line length [Expected: 80; Actual: 99]
README.md:485:81 MD013/line-length Line length [Expected: 80; Actual: 93]
README.md:486:81 MD013/line-length Line length [Expected: 80; Actual: 187]
README.md:487 MD032/blanks-around-lists Lists should be surrounded by blank lines [Context: "affiliation if these cannot be..."]
README.md:488 MD031/blanks-around-fences Fenced code blocks should be surrounded by blank lines [Context: "```json"]
README.md:505 MD031/blanks-around-fences Fenced code blocks should be surrounded by blank lines [Context: "```"]
README.md:506:1 MD029/ol-prefix Ordered list item prefix [Expected: 1; Actual: 7; Style: 1/1/1]
README.md:506 MD032/blanks-around-lists Lists should be surrounded by blank lines [Context: "7. [Create a PR](#create-a-pap..."]
README.md:508:81 MD013/line-length Line length [Expected: 80; Actual: 91]
review_criteria.md:27:81 MD013/line-length Line length [Expected: 80; Actual: 115]
review_criteria.md:75 MD025/single-title/single-h1 Multiple top-level headings in the same document [Context: "# Suggestions for Writing Grea..."]
review_criteria.md:77:1 MD004/ul-style Unordered list style [Expected: dash; Actual: asterisk]
review_criteria.md:77:81 MD013/line-length Line length [Expected: 80; Actual: 86]
review_criteria.md:79:1 MD004/ul-style Unordered list style [Expected: dash; Actual: asterisk]
review_criteria.md:79:81 MD013/line-length Line length [Expected: 80; Actual: 87]
review_criteria.md:83:1 MD004/ul-style Unordered list style [Expected: dash; Actual: asterisk]
review_criteria.md:84:1 MD004/ul-style Unordered list style [Expected: dash; Actual: asterisk]
review_criteria.md:84:81 MD013/line-length Line length [Expected: 80; Actual: 90]
review_criteria.md:87:1 MD004/ul-style Unordered list style [Expected: dash; Actual: asterisk]
review_criteria.md:88:81 MD013/line-length Line length [Expected: 80; Actual: 86]
review_criteria.md:91:1 MD004/ul-style Unordered list style [Expected: dash; Actual: asterisk]
review_criteria.md:91:81 MD013/line-length Line length [Expected: 80; Actual: 91]
review_criteria.md:92:81 MD013/line-length Line length [Expected: 80; Actual: 92]
review_criteria.md:94:1 MD004/ul-style Unordered list style [Expected: dash; Actual: asterisk]
review_criteria.md:96:1 MD004/ul-style Unordered list style [Expected: dash; Actual: asterisk]
review_criteria.md:96:81 MD013/line-length Line length [Expected: 80; Actual: 88]
review_criteria.md:97:81 MD013/line-length Line length [Expected: 80; Actual: 91]
review_criteria.md:98:81 MD013/line-length Line length [Expected: 80; Actual: 91]
review_criteria.md:101 MD025/single-title/single-h1 Multiple top-level headings in the same document [Context: "# Proceedings Committee"]
review_criteria.md:103:1 MD004/ul-style Unordered list style [Expected: dash; Actual: asterisk]
review_criteria.md:103:81 MD013/line-length Line length [Expected: 80; Actual: 238] maybe we should do a follow up PR with it and |
Closing to try pre-commit.ci |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. It seems run fine 👍
A note here for @scipy-conference/proceedings given Slack discussion. After this PR is merged this should get backported to the edit: (Though GitHub won't notify new people tagged in an edit, so I've requested review) This should have been @scipy-conference/2024-proceedings. (Maybe the old team should get retired to avoid confusion.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there some reason this branch seeks to make so many minor formatting and other changes to so many files other than just adding .pre-commit-config.yaml ?
@cbcunc Please read the PR body where this is explained: #1017 (comment) |
Does not explain why it needs to run on --all-files. |
@scipy-conference/2024-proceedings Okay, let me try to unpack
in much more detail then.
I think that should explain why As I also mentioned, I understand that this is a large change, and people generally don't want to have to deal with this noise when using I did not include a |
Be aware that when this is backported to the |
@scipy-conference/2024-proceedings I see from PR #925 that pre-commit.ci has been disabled as it doesn't show up in the checks and when you look at the pre-commit.ci page for the repo (https://results.pre-commit.ci/repo/github/1825629) it was disabled before the last pushes #925 (commits). Is there still interest in using pre-commit.ci (even if not while the 2024 proceedings are still in process)? |
Yes, after we're done with the 2024 process. |
In my original comment I should have mentioned incorporating use of pre-commit.ci after the 2024 proceedings are published. Thank you, @matthewfeickert and @tkoyama010, for your work on this. |
Resolves #1016
schema validation
The second commit in this PR is huge, but it is just applying the results of the added pre-commit hooks via
as @tkoyama010 has started setting up pre-commit.ci (#1016 (comment)) and pre-commit.ci will run the equivalent of
pre-commit run --all-files
on each commit. This commit can get ignored automatically from GitHub'sgit blame
on diffs via adding it to a.git-blame-ignore-revs
(c.f. https://github.com/matplotlib/matplotlib/blob/1e8ea2f6ac758b331fb8b6838d69822424e86cb4/.git-blame-ignore-revs for an example).