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

build(docs): auto alphabetize .spelling file #12521

Merged
merged 3 commits into from
Jan 17, 2024

Conversation

agilgur5
Copy link
Member

@agilgur5 agilgur5 commented Jan 14, 2024

Fixes #12360 (comment)
Related to #11814 (comment)

Motivation

We've had a few different attempts to alphabetize this at different times, but manual efforts mean that things can get undone unfortunately easily. So I wrote a tiny script to automate alphabetizing it

Modifications

  • run a tiny shell script as part of make docs-spellcheck to alphabetize the file

    • currently made it case-sensitive sorted as that was a smaller diff / more consistent with the existing ordering
      • but could make it case-insensitive if we wanted to
  • clarify / shorten comments in the .spelling file (was previously verbatim copy+pasted from its README)

  • slightly rewrite comments / naming in Docs GHA Workflow step to account for this additional "linter"

Verification

  1. Manually ran the script locally a bunch of times
  2. Manually ran as part of the Makefile, where the $(shell ...) was necessary
  3. Manually ran in both devcontainer and native on macOS
    • In particular, some simpler commands like { head -n 5; sort; } or { sed -u 5q; sort; } did not seem to work on my Mac, so I went with the awk variant
      • or a cat <(head -n 5 .spelling) <(tail -n +6 .spelling | sort | uniq)
    • similarly, I threw a | tee in there as just redirecting the output with > .spelling would cause an empty file
    • portability 😕 that took way too long to test as a result 😕 big reason why I'm not big on shell

- run a tiny shell script as part of `make docs-spellcheck` to alphabetize the file
  - currently made it case-senstive sorted as that was a smaller diff / more consistent with the existing ordering
    - but could make it case-insensitive if we wanted to

- slightly rewrite comments / naming in Docs GHA Workflow step to account for this additional "linter"

Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
@agilgur5 agilgur5 added area/docs Incorrect, missing, or mistakes in docs area/build Build or GithubAction/CI issues labels Jan 14, 2024
@agilgur5
Copy link
Member Author

Windows unit test randomly failed, but that is a flake and not currently required to pass CI

Makefile Outdated Show resolved Hide resolved
- the previous text was direclty taken from [the README](https://github.com/lukeapage/node-markdown-spellcheck/blob/78ad8e3c21fe982a1e4d1fc1fbf421fb57c9739b/readme.md?plain=1#L106)
  - we don't use the ` - filename` config part (and nor am I sure how it works), so remove that
  - comment that `# ` is a comment is self-explanatory / redundant if there is a single comment
  - clarify that we use this as a dictionary specifically and no other config

Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
@agilgur5 agilgur5 requested a review from Joibel January 17, 2024 19:27
@terrytangyuan terrytangyuan merged commit df79ce4 into argoproj:main Jan 17, 2024
27 checks passed
@agilgur5 agilgur5 deleted the build-docs-alphabetize-spelling branch January 17, 2024 23:39
agilgur5 added a commit that referenced this pull request May 11, 2024
Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
(cherry picked from commit df79ce4)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/build Build or GithubAction/CI issues area/docs Incorrect, missing, or mistakes in docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants