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

[INFRA] Add "codespell" tool to CI checks to catch typos sooner #873

Merged
merged 4 commits into from
Sep 13, 2021

Conversation

DimitriPapadopoulos
Copy link
Collaborator

@DimitriPapadopoulos DimitriPapadopoulos commented Sep 12, 2021

New typos found by latest version of codespell.

A couple other style issues.

New typos found by latest version of codespell.
Mark executable Python scripts as executable Python in Git. While Git is
not perfect in how it treats changes in file permissions, I feel this is
better than nothing.
Copy link
Collaborator

@Remi-Gau Remi-Gau left a comment

Choose a reason for hiding this comment

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

LGTM

Personal note: learn how to spell "dictionary"

Copy link
Member

@sappelhoff sappelhoff left a comment

Choose a reason for hiding this comment

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

Thanks @DimitriPapadopoulos I have two comments:

You have touched some files without content changes, like this one (screenshot):

image

What's that about?

Secondly, do you think it'd make sense to integrate codespell into our CI (if possible)?

@DimitriPapadopoulos
Copy link
Collaborator Author

DimitriPapadopoulos commented Sep 13, 2021

Ah, you're right. I ran chmod -x on that file because it should not be executable (does not start #!). See the comments in the different commits.

I am trying to integrate codespell in the CI process of different projects, but I'm not there yet.

@DimitriPapadopoulos
Copy link
Collaborator Author

DimitriPapadopoulos commented Sep 13, 2021

Actually, I'm already there for GitHub, but not for GitLab.

I have added codespell to GitHub CI. The problem is with some residual false-positives:

./tools/schemacode/schema.py:40: fo ==> of, for
./tools/schemacode/schema.py:41: fo ==> of, for
./src/01-introduction.md:119: TE ==> THE, BE, WE, TO
./src/99-appendices/08-coordinate-systems.md:110: ALS ==> ALSO
./src/99-appendices/08-coordinate-systems.md:112: ALS ==> ALSO
./src/99-appendices/08-coordinate-systems.md:113: ALS ==> ALSO
./src/99-appendices/08-coordinate-systems.md:133: ALS ==> ALSO
./src/99-appendices/08-coordinate-systems.md:137: ALS ==> ALSO
./src/schema/metadata/EchoTime.yaml:4: TE ==> THE, BE, WE, TO
./pdf_build_src/process_markdowns.py:443: fo ==> of, for
./pdf_build_src/process_markdowns.py:444: fo ==> of, for
./pdf_build_src/process_markdowns.py:467: fo ==> of, for
./pdf_build_src/process_markdowns.py:468: fo ==> of, for

I'm not certain what the best strategy is. Accept all these small variables names as valid, and perhaps skip future actual errors?

@sappelhoff
Copy link
Member

Cool!

Accept all these small variables names as valid, and perhaps skip future actual errors?

I'd say that's an improvement over the status quo 👍

@DimitriPapadopoulos
Copy link
Collaborator Author

Done.

@DimitriPapadopoulos
Copy link
Collaborator Author

I hope I have got the on: part right

@sappelhoff sappelhoff changed the title [DOC] Typos found by codespell [INFRA] Add "codespell" tool to CI checks to catch typos sooner Sep 13, 2021
Copy link
Member

@sappelhoff sappelhoff left a comment

Choose a reason for hiding this comment

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

Great, thanks a lot @DimitriPapadopoulos

@sappelhoff sappelhoff merged commit 2050b32 into bids-standard:master Sep 13, 2021
@DimitriPapadopoulos DimitriPapadopoulos deleted the codespell branch September 13, 2021 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants