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

Proposal: automatically spell check and fix errors in release notes and changelog #62153

Open
wants to merge 4 commits into
base: trunk
Choose a base branch
from

Conversation

vcanales
Copy link
Member

@vcanales vcanales commented May 30, 2024

What?

This PR introduces a spell-checking step for release notes and change log in the GitHub Actions workflow. The action uses crate-ci's typos package under the hood, a CLI utility for spell checking written in rust.

Why?

Adding a spell check ensures that the release notes are free of typos and grammatical errors, helping maintain a professional standard. By automating this process, we can reduce the likelihood of errors slipping through manually written documentation.

The change log currently has a lot of spelling errors — a sample:

error: `prefered` should be `preferred`
  --> changelog.txt:37845:19
      |
37845 | *   Expand the [**prefered-reduced-motion** support](https://github.com/WordPress/gutenberg/pull/15850) to all the animations.
      |                   ^^^^^^^^
      |
error: `placehoder` should be `placeholder`
  --> changelog.txt:38589:38
      |
38589 | - Update the default [block appender placehoder](https://github.com/WordPress/gutenberg/pull/13880).
      |                                      ^^^^^^^^^^
      |
error: `infinte` should be `infinite`
  --> changelog.txt:39125:16
      |
39125 | * Fix RichText infinte rerendering
      |                ^^^^^^^
      |
error: `syle` should be `style`
  --> changelog.txt:39301:25
      |
39301 | * Fixes potential theme syle.css clash.
      |                         ^^^^
      |
error: `useage` should be `usage`
  --> changelog.txt:39352:22
      |
39352 | * Remove findDOMNode useage from NavigableToolbar.
      |                      ^^^^^^
      |
error: `visibile` should be `visible`
  --> changelog.txt:41144:7
      |
41144 | * Add visibile text to gallery "add item" button for accessibility.
      |       ^^^^^^^^
      |
error: `transfering` should be `transferring`
  --> changelog.txt:41368:24
      |
41368 | * Fix issue with focus transfering between citation and content.
      |                        ^^^^^^^^^^^
      |
error: `bootstraping` should be `bootstrapping`
  --> changelog.txt:41430:175
      |
41430 | * Refactor Default Colors and the ColorPalette component. Moves the default colors to the frontend making the Editor script more reusable without the need of the server-side bootstraping.
      |                                                                                                                                                                               ^^^^^^^^^^^^
      |
error: `excesive` should be `excessive`
  --> changelog.txt:41505:71
      |
41505 | * Replace global hover state with local component state. This reduces excesive dispatching of mouse events.

This is understandable, and adding this steps makes it so that errors can be corrected without having to depend on the contributor editing their PR title.

How?

The workflow file .github/workflows/build-plugin-zip.yml has been updated to include a new step utilizing the crate-ci/typos action. This step checks the release-notes.txt file for typos and automatically writes changes to correct any found errors.

Testing Instructions

  1. Fork this repo; the following steps should be taken in the fork.
  2. Trigger the GitHub Actions workflow by pushing a change to the repository.
  3. Verify that the Spell check release notes step runs successfully in the workflow.
  4. Check the release-notes.txt file for any automated corrections made by the spell check.
This will catch spelling errors, specially in the changelog, where there
are many. This will help us have a better changelog without having to
ask contributors to spell check the titles of their PRs.
@vcanales vcanales marked this pull request as ready for review May 30, 2024 18:11
@vcanales vcanales requested a review from desrosj as a code owner May 30, 2024 18:11
Copy link

github-actions bot commented May 30, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: vcanales <vcanales@git.wordpress.org>
Co-authored-by: desrosj <desrosj@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@vcanales vcanales added [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. [Type] Enhancement A suggestion for improvement. labels May 30, 2024
@vcanales vcanales changed the title Proposal: automatically spell check and fix errors in release notes May 30, 2024
@vcanales vcanales removed the [Type] Enhancement A suggestion for improvement. label May 30, 2024
@vcanales vcanales requested a review from ockham May 30, 2024 18:23
@desrosj
Copy link
Contributor

desrosj commented Jun 5, 2024

Thanks for this, @vcanales!

I love this as a great safe guard. But I think that I am against automatically fixing any typos. Without human review of any flagged misspellings, we risk incorrect adjustments introducing errors instead of fixing (the opposite of the intention of this PR).

The first question that came to mind was how to override the suggested spellings. Where this is a code base, there will likely be "incorrect" spellings that are desired. It looks like there is a way to maintain a false-positive list for a project. I think it would be good to configure this with just "WordPress" in there for now.

It's not clear to me if we can enforce proper casing for words. If so, then that would be nice to add as well (WordPress vs. wordpress or Wordpress, etc.).

And final thought: would it make more sense to run this more regularly? Or earlier in the process? I know this is specifically for the release notes, but holding up the final release process to fix these at that time is not ideal (those releases can take extended amounts of time if other issues are discovered).

@vcanales
Copy link
Member Author

vcanales commented Jun 10, 2024

It looks like there is a way to maintain a false-positive list for a project. I think it would be good to configure this with just "WordPress" in there for now.

From a search of f87b04c, it seems like the correct "WordPress" identifier is not throwing false positives, probably because it's being interpreted as a Proper noun. We can add it to be safe as well.

It's not clear to me if we can enforce proper casing for words. If so, then that would be nice to add as well (WordPress vs. wordpress or Wordpress, etc.).

Seems like we can, but we'd have to add a specific rule for each incorrect spelling.

As an example, on this content:

wordpress WordPress wordPress woRdPress
Wordpress
wordpres WordPres

And these rules:

[default.extend-identifiers]
wordPress = "WordPress"
Wordpress = "WordPress"
WordPres  = "WordPress"

typos returns:

➜  gutenberg git:(try/spellcheck-release-notes) ✗ typos wordpresses.txt
error: `wordPress` should be `WordPress`
  --> wordpresses.txt:1:21
  |
1 | wordpress WordPress wordPress woRdPress
  |                     ^^^^^^^^^
  |
error: `Wordpress` should be `WordPress`
  --> wordpresses.txt:2:1
  |
2 | Wordpress
  | ^^^^^^^^^
  |
error: `wordpres` should be `wordpress`
  --> wordpresses.txt:3:1
  |
3 | wordpres
  | ^^^^^^^^
  |

For some reason, it misses "WordPres" in the same conditions as above, but adding a rule for it makes it work.

The unreasonably bad "woRdPress" is ignored, but should probably be caught in the human review of the PR anyway.

But I think that I am against automatically fixing any typos. Without human review of any flagged misspellings, we risk incorrect adjustments introducing errors instead of fixing (the opposite of the intention of this PR).
And final thought: would it make more sense to run this more regularly? Or earlier in the process? I know this is specifically for the release notes, but holding up the final release process to fix these at that time is not ideal (those releases can take extended amounts of time if other issues are discovered).

I kind of agree; I went for an auto-fix because I initially thought that the friction by a human review might be enough to turn this into a nuisance rather than an improvement, but perhaps in order to make this a manual process instead we could have a bot comment on PRs with typos on their titles, asking the author to fix them before merging, since the PR title is what makes it into the Change Log.

That being said, I think that incorrect adjustments would be fewer than the actual small spelling errors currently present. For reference, I've added what the auto-fix would do to our current change log in f87b04c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests.
2 participants