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

Spell check action for markdown documentation #8675

Merged
merged 4 commits into from May 16, 2021

Conversation

jsoref
Copy link
Contributor

@jsoref jsoref commented May 14, 2021

This is a πŸ™‹ feature or enhancement.

Summary

Add the check-spelling action limited to docs/ directory .md files.

Context

Based on @DirtyF #8661 (comment)

Note: as currently configured, pushes get a comment, and PRs get another, this may feel like duplication, but it enables contributors to enable actions in their forks and get feedback before creating a PR.

I'm still looking into ways to address this duplication.

It's software, as with everything else, so there are bugs
  • There's one glitch where the comment has a duplicated word, it's fixed in the prerelease branch, but generally I prefer for public projects to use released versions).
  • This version has a flag guarded feature for updating PRs, but it shouldn't be used in public repositories as it has a bug where it'll comment prematurely for things not addressed to the bot (this is also fixed in prerelease).

There's one more feature that I'm working on which I'd like to finalize before my next release (hopefully in a week or so, I failed to notice where I was in the calendar, which is very ironic, as I've been literally counting the days).

Landing advice

For any fast moving repository, the initial merge tends to be a bit rocky. Between when #8661 was written and when it was merged, two additional typos were added just to the documentation directory.

The person merging this should enable actions in their fork and test the merged result (the easiest thing to do is make this branch the repository's default and create a PR from jekyll/jekyll master into their repository (with spell-check as the default) -- you can use check-spelling/jekyll as that repository if you like, as it's set up (but I won't be able to make changes to it, so you're probably better served using your own fork). If it gets a βœ… from check-spelling, then it's safe to merge. If it gets a ❌ , there will be a comment identifying the words that are unrecognized. They can be added to expect.txt if they're intended, or fixed if they're errors.

It's best to do that before merging. If you merge with a ❌ then anyone who uses that landing point for their branch point will get slightly confusing ❌ feedback on their branch. (Their PR could still report a βœ… if master fixed because of the configuration of the workflow, but the confusion is confusing.)


Sadly, I won't be able to provide much help between now and Wednesday. Generally I'm fairly responsive (w/in 2 days).

If you have a communications channel that you use and want help w/ the landing, let me know (again, I can help from Wednesday). Have a good weekend/week.

Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
This is limited to `.md` files in the `docs/` directory by
an entry in the `only.txt` file.

If that file is empty/removed, all files not otherwise excluded
would be checked.

To check additional paths, you can add additional lines to match
them.

Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
@jsoref
Copy link
Contributor Author

jsoref commented May 14, 2021

The force push was to fix a grammatical botch in the commit message 😊 . This is a spell checker, not a grammar checker, and I'm in a hurry (my day ends momentarily). Sorry about that...

Copy link
Member

@DirtyF DirtyF left a comment

Choose a reason for hiding this comment

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

Let's try it and see how it feels! Thank you very much for this useful automation @jsoref, hope it will reduce the numbers of PR to fix typos πŸ™

.github/actions/spelling/excludes.txt Outdated Show resolved Hide resolved
@DirtyF
Copy link
Member

DirtyF commented May 16, 2021

@jekyllbot: merge +dev

@jekyllbot jekyllbot merged commit bdad4f2 into jekyll:master May 16, 2021
jekyllbot added a commit that referenced this pull request May 16, 2021
@jsoref jsoref deleted the spell-check branch May 16, 2021 17:25
@jekyll jekyll locked and limited conversation to collaborators May 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants