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

codespell: config, workflow (fail if typos introduced) and some typos fixed #1083

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

yarikoptic
Copy link

The trickiest part was to ignore the embedded in notebooks images and videos - the most of the ways I saw so far ;-)

I mass ignored all detected short words, but may be many of them aren't even needed to be ignored, but some are variable names (case ignored).

@spirosChv
Copy link
Collaborator

@yarikoptic Unfortunately, your PR cannot pass our CI checks. Generally, we used a specific process to ensure that all notebooks are working (the full, student and instructor versions). For projects, we use a different process. Here, you have committed changes in instructor and student versions, which are handled by our CI automatically. Please refer to our contributing guidelines: https://github.com/NeuromatchAcademy/course-content/blob/main/.github/contributing.md

As you have corrected a lot of typos (which is great) maybe I can add you to the maintenance team to make those changes on our repo directly. PRs from forks have lead to several issues in the past. Send me an email if you want to contribute. Thank you very much for your effort and time!

@yarikoptic
Copy link
Author

ok, I sent email to the email address at forth.gr discovered elsewhere (email in git history is not informative ;) ) .

FWIW: I also force-pushed to address one missed false positive. reviewed entire diff -- should be good now AFAIK,

@iamzoltan
Copy link
Contributor

@yarikoptic are you still working on this?

@yarikoptic
Copy link
Author

eh, since required more of a dance, I have went on with my other dues and missed an invitation to join from @spirosChv who kindly provided instructions via email on how to proceed (seems to not really requiring to join, should be able to do via PRs from forks)... hopefully I will find some time to get back to this, but no promises -- anyone is welcome to pick up the effort -- feel welcome to close this PR.

@iamzoltan
Copy link
Contributor

iamzoltan commented Jul 18, 2024

@yarikoptic Can we revamp this? Maybe resolve conflicts and only edit the tutorials (the student and instructor versions get generated in the processing pipeline)

We can change the base to a PR that I will create in the repo so that the processing will run internally.

Signed-off-by: Yaroslav Halchenko <[email protected]>
Signed-off-by: Yaroslav Halchenko <[email protected]>
Signed-off-by: Yaroslav Halchenko <[email protected]>
Signed-off-by: Yaroslav Halchenko <[email protected]>
Signed-off-by: Yaroslav Halchenko <[email protected]>
Signed-off-by: Yaroslav Halchenko <[email protected]>
=== Do not change lines below ===
{
 "chain": [],
 "cmd": "codespell -w -i 3 -C 2",
 "exit": 0,
 "extra_inputs": [],
 "inputs": [],
 "outputs": [],
 "pwd": "."
}
^^^ Do not change lines above ^^^

Signed-off-by: Yaroslav Halchenko <[email protected]>
Signed-off-by: Yaroslav Halchenko <[email protected]>
=== Do not change lines below ===
{
 "chain": [],
 "cmd": "codespell -w",
 "exit": 0,
 "extra_inputs": [],
 "inputs": [],
 "outputs": [],
 "pwd": "."
}
^^^ Do not change lines above ^^^
@yarikoptic
Copy link
Author

FWIW, ignored one more name (Christina Savin), rebased/reran fixup. I didn't leave projects out yet - seems not that many/reasonable to review.

@iamzoltan
Copy link
Contributor

great thanks! Are you going to review projects too?

@yarikoptic
Copy link
Author

I think I did fix them up too ATM, have a look in the diff.

@iamzoltan
Copy link
Contributor

will do, thanks!

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