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

Add Codespell action #32

Merged
merged 3 commits into from Aug 17, 2020
Merged

Add Codespell action #32

merged 3 commits into from Aug 17, 2020

Conversation

peternewman
Copy link
Contributor

No description provided.

@peternewman
Copy link
Contributor Author

This has found a number of errors:
https://github.com/peternewman/niceware/actions/runs/197104926

But only in browser/niceware.js not lib/wordlist.js, your build doesn't seem to use wordlist though (from what I can see):

"build": "browserify lib/main.js -o browser/niceware.js",

@diracdeltas diracdeltas self-assigned this Aug 6, 2020
@diracdeltas
Copy link
Owner

thanks for doing this! unfortunately all of these spelling errors (except 'keypair' which i contend is a real word) were generated by browserify, a tool which this repo uses to autogenerate browser code. as such, i can't fix the spelling errors without them being overriden every time a new build is generated.

@diracdeltas diracdeltas closed this Aug 7, 2020
@peternewman
Copy link
Contributor Author

peternewman commented Aug 7, 2020

thanks for doing this! unfortunately all of these spelling errors (except 'keypair' which i contend is a real word) were generated by browserify, a tool which this repo uses to autogenerate browser code. as such, i can't fix the spelling errors without them being overriden every time a new build is generated.

🤦 oops @diracdeltas , I need to work out what I first spotted and unpick it (edit, see below). I've opened an upstream issue to fix browserify ( browserify/browserify#1975 ), although those typos don't exist in it (although I found a lot of other upstream in feross/buffer#269 and defunctzombie/node-process#89 ). In the meantime, I've just skipped that file anyway. See for example:
https://github.com/peternewman/niceware/actions/runs/198897858

Incidentally something like this looks like it could probably minify that generated output at the bottom I think:
https://www.npmjs.com/package/tinyify

(except 'keypair' which i contend is a real word)

Do you have some source or links for that? A Google finds AWS, Java and Wikipedia all referring to two words. Outscale (whatever that is) is one of the few things I could find which uses one word. We could also skip it though like other words.

codespell-project/codespell#1613 was what started me at this, although after a bit more digging it seems to be a rare but real word so I've left it for now.

@peternewman
Copy link
Contributor Author

@diracdeltas are you interested in merging it now it only checks your actual code?

As I mentioned, if you've got some detail on keypair, it would be great to correct the dictionary, or if it's just personal preference it's also trivial to skip in your repo check?

@diracdeltas diracdeltas reopened this Aug 17, 2020
@diracdeltas
Copy link
Owner

As I mentioned, if you've got some detail on keypair, it would be great to correct the dictionary, or if it's just personal preference it's also trivial to skip in your repo check?

it does seem used less frequently than i thought so i'm happy to use the more common two-word spelling. i will merge this and then fix it.

thanks for the PR!

@diracdeltas diracdeltas merged commit 42521b2 into diracdeltas:master Aug 17, 2020
@peternewman
Copy link
Contributor Author

As I mentioned, if you've got some detail on keypair, it would be great to correct the dictionary, or if it's just personal preference it's also trivial to skip in your repo check?

it does seem used less frequently than i thought so i'm happy to use the more common two-word spelling. i will merge this and then fix it.

Great, we'll leave our dictionary as-is then.

thanks for the PR!

No worries.

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.

None yet

2 participants