-
Notifications
You must be signed in to change notification settings - Fork 14
CI: Add Non-Blocking Notebook Spell Check with PR Commenting #277
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
Conversation
|
This is actually great. I ran the spellchecker only on markdown cells instead of the code cells. I'll fix these typos in the other PR. |
a417ff5 to
3852c90
Compare
e4354c2 to
b12ed32
Compare
|
Went thought the action, nice, quite like the logic! Looks good |
daquinteroflex
left a comment
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.
Looks good broadly!
Spell Check ReportAdjointPlugin12LightExtractor.ipynb: Autograd12LightExtractor.ipynb: Checked 47 notebook(s). Found spelling errors in 2 file(s). |
47d8306 to
8638f03
Compare
|
I ran the spell check on all notebooks and fixed the remaining typos. So everything should be clean for now. The remaining spell check errors are false positives that we can ignore. They won't pop up again unless the respective notebooks are changed. |
tomflexcompute
left a comment
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.
Great addition to the workflow!
Spelling errors within Markdown cells or comments in notebooks currently go mostly unnoticed. We want to catch these errors.
The aim of this PR is to inform developers rather than block merging, as there can be false positives in the spell check.
Changes Implemented:
spellcheck.py: Executed as part of thenotebooks-lintingworkflow..ipynbfiles that have been modified within the context of the specific pull request. This is achieved using thetj-actions/changed-filesaction.continue-on-error: true. This means that even ifspellcheck.pyfinds errors and exits with a non-zero status code, the overall CI workflow will not fail. Linting failures will still cause the workflow to fail as before.spellcheck.pydetects any spelling errors or encounters issues processing a file, its formatted output is captured and posted as a comment directly on the PR, see here for example.Closes #207