doc: Fix some misspellings #17351

RandyMcMillan commented at 7:20 AM on November 2, 2019: contributor

Here is a more thorough lint-spelling update. This PR takes care of easy to fix spelling errors to clean up the linting stages. There are misspellings coded into the functional tests. That is a whole separate job within itself.

DrahtBot added the label Build system on Nov 2, 2019

DrahtBot added the label Docs on Nov 2, 2019

DrahtBot added the label GUI on Nov 2, 2019

DrahtBot added the label P2P on Nov 2, 2019

DrahtBot added the label RPC/REST/ZMQ on Nov 2, 2019

DrahtBot added the label Scripts and tools on Nov 2, 2019

DrahtBot added the label Tests on Nov 2, 2019

DrahtBot added the label Validation on Nov 2, 2019

DrahtBot commented at 9:02 AM on November 2, 2019: member

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#16519 (guix: Change manifest to use channels and inferiors by dongcarl)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

hebasto commented at 9:34 AM on November 2, 2019: member

Concept ACK ac6d97fc4a1cd04db20fd773a05cd1eb0eebc373, tested on Linux Mint 19.2:

$ pip3 show codespell | grep Version
Version: 1.16.0

$ ./test/lint/lint-spelling.sh 
src/test/base32_tests.cpp:14: fo  ==> of, for
src/test/base64_tests.cpp:14: fo  ==> of, for
test/lint/lint-locale-dependence.sh:100: stoll  ==> still
^ Warning: codespell identified likely spelling errors. Any false positives? Add them to the list of ignored words in test/lint/lint-spelling.ignore-words.txt

EDITED: @RandyMcMillan FWIW: Status: Draft means PR is not ready to be reviewed.

EDITED 2: IMHO, the commit message could be more expressive like test: fix all typos ;)

laanwj removed the label Build system on Nov 2, 2019

laanwj removed the label GUI on Nov 2, 2019

laanwj removed the label P2P on Nov 2, 2019

laanwj removed the label RPC/REST/ZMQ on Nov 2, 2019

laanwj removed the label Scripts and tools on Nov 2, 2019

laanwj removed the label Tests on Nov 2, 2019

laanwj removed the label Validation on Nov 2, 2019

laanwj commented at 10:17 AM on November 2, 2019: member

Concept ACK, but from now on, stop opening multiple PRs for everything. If you need to figure out how github works, please experiment with a repository of your own instead of our production repo.

hebasto commented at 10:56 AM on November 2, 2019: member

These words could be dropped from ignore list: https://github.com/bitcoin/bitcoin/blob/9641366950276da88af626d0898676195df8d83a/test/lint/lint-spelling.ignore-words.txt#L1

https://github.com/bitcoin/bitcoin/blob/9641366950276da88af626d0898676195df8d83a/test/lint/lint-spelling.ignore-words.txt#L6

mut could be dropped with renaming s/mut/m_mutex/ in threadinterrupt.{h|cpp}.

useable, unparseable, cachable could be dropped with proper fixes in the code base. However, I'm not sure about other devs opinion ;)

homogenous could be dropped with a proper fix in tinyformat.h. Also please see #12716 (review).

in test/lint/lint-spelling.sh:18 in ac6d97fc4a outdated

  14 | @@ -15,6 +15,6 @@ if ! command -v codespell > /dev/null; then
  15 |  fi
  16 |  
  17 |  IGNORE_WORDS_FILE=test/lint/lint-spelling.ignore-words.txt
  18 | -if ! codespell --check-filenames --disable-colors --quiet-level=7 --ignore-words=${IGNORE_WORDS_FILE} $(git ls-files -- ":(exclude)build-aux/m4/" ":(exclude)contrib/seeds/*.txt" ":(exclude)depends/" ":(exclude)doc/release-notes/" ":(exclude)src/leveldb/" ":(exclude)src/qt/locale/" ":(exclude)src/secp256k1/" ":(exclude)src/univalue/"); then
  19 | +if ! codespell --check-filenames --disable-colors --quiet-level=7 --ignore-words=${IGNORE_WORDS_FILE} $(git ls-files -- ":(exclude)build-aux/m4/" ":(exclude)contrib/seeds/*.txt" ":(exclude)depends/" ":(exclude)doc/release-notes/" ":(exclude)src/leveldb/" ":(exclude)src/qt/locale/" ":(exclude)src/qt/bitcoin_locale.qrc" ":(exclude)src/secp256k1/" ":(exclude)src/univalue/"); then

practicalswift commented at 11:20 AM on November 2, 2019:

Nit: Could replace ":(exclude)src/qt/bitcoin_locale.qrc" with ":(exclude)*.qrc"

RandyMcMillan commented at 12:24 AM on November 3, 2019:

Thanks @practicalswift - awesome suggestion.

RandyMcMillan commented at 1:07 AM on November 3, 2019:

Included this suggestion in the commit. thanks again.

practicalswift commented at 11:21 AM on November 2, 2019: contributor

ACK ac6d97fc4a1cd04db20fd773a05cd1eb0eebc373 modulo addressing of the feedback from @hebasto :)

hebasto commented at 4:04 PM on November 2, 2019: member

Here are some recent similar PRs:

Maybe it is better to make such a PR once only per major and/or minor release (as part of release process)?

MarcoFalke renamed this:
~~build:lint eliminate some lint spelling alerts~~
doc: Fix some misspellings
on Nov 2, 2019

RandyMcMillan commented at 12:23 AM on November 3, 2019: contributor

@laanwj - I apologize for the PR noise. For some reason I can't reopen PRs. It is probably something I did. (forced push setting in your admin settings) - I have since figured out my workflow. So if it is an admin setting on your side - I apologize and have since implemented my own build environment and will only create PRs once I have tested my changes personally FIRST! Again sincere apology. ditto to @MarcoFalke and the rest of the members - @hebasto @practicalswift @dongcarl @promag etc...

RandyMcMillan commented at 1:10 AM on November 3, 2019: contributor

These words could be dropped from ignore list:

https://github.com/bitcoin/bitcoin/blob/9641366950276da88af626d0898676195df8d83a/test/lint/lint-spelling.ignore-words.txt#L1

https://github.com/bitcoin/bitcoin/blob/9641366950276da88af626d0898676195df8d83a/test/lint/lint-spelling.ignore-words.txt#L6

mut could be dropped with renaming s/mut/m_mutex/ in threadinterrupt.{h|cpp}. @hebasto,

I left the words you were unsure about out until we have more feed back.

I also removed /bitcoin/contrib/guix/manifest.scm from the commit.

If @dongcarl's change gets merged he will have killed two birds. REF: https://github.com/bitcoin/bitcoin/pull/16519/commits/d4e0694feb4c0ee2f7eb9db6b3f56efbc5a5bffc

useable, unparseable, cachable could be dropped with proper fixes in the code base. However, I'm not sure about other devs opinion ;)

homogenous could be dropped with a proper fix in tinyformat.h. Also please see #12716 (comment).

practicalswift commented at 6:06 AM on November 3, 2019: contributor

ACK d17d888551d928642fcf76cbf64e7aa278426143

hebasto commented at 7:57 AM on November 3, 2019: member

Tested d17d888551d928642fcf76cbf64e7aa278426143:

$ ./test/lint/lint-spelling.sh | grep mut
src/threadinterrupt.cpp:25: mut  ==> must, mutt, moot
src/threadinterrupt.cpp:33: mut  ==> must, mutt, moot
src/threadinterrupt.h:32: mut  ==> must, mutt, moot

@RandyMcMillan Did you forget to rename mut to m_mutex as suggested?

I also removed /bitcoin/contrib/guix/manifest.scm from the commit.

Why? The goal of the mentioned commit is orthogonal to the goal of this PR. I'd also prefer a complete solution over partial one, if it is feasible.

There is a misspelling in the commit message "doc:Fix some mispellings" ;)

RandyMcMillan commented at 9:20 AM on November 3, 2019: contributor

$ ./test/lint/lint-spelling.sh | grep mut

@hebasto - speaking of grep - we need to add an alias to ggrep in the Mac environment. Brew doesnt do it automatically - this will take care on a linting issue in the Mac build.

promag commented at 9:29 PM on November 3, 2019: member

Concept ACK, no longer draft?

CI failing with, for instance:

Running test/util/bitcoin-util-test.py...
/usr/bin/python3.6 ../test/util/bitcoin-util-test.py
2019-11-03 20:06:22,888 - ERROR - Error mismatch:
Expected: error: Uncompressed pubkeys are not usable for SegWit outputs
Received: error: Uncompressed pubkeys are not useable for SegWit outputs

RandyMcMillan commented at 12:25 AM on November 4, 2019: contributor

NOTE: There are misspellings built into the functional tests.

That is a whole job with in itself. I had to pull some changes out.

Sorry about the extra builds.

I guess that is why people don't try to fix the linting.

Preliminary test build: https://travis-ci.org/RandyMcMillan/bitcoin/builds/606965245 @hebasto @promag Yes, I believe it is ready.

I appreciate your patience and feedback.

RandyMcMillan marked this as ready for review on Nov 4, 2019

RandyMcMillan requested review from hebasto on Nov 4, 2019

RandyMcMillan requested review from practicalswift on Nov 4, 2019

hebasto commented at 6:49 AM on November 4, 2019: member

@RandyMcMillan Please do not touch files in doc/man/ as they are generated by a script, and historical release notes in doc/release-notes/.

doc: Fix some misspellings ac831339cb

practicalswift approved

practicalswift commented at 9:34 AM on November 4, 2019: contributor

ACK ac831339cbfa65b1f7576c53b5d9a94841db9868 -- diff looks correct

RandyMcMillan commented at 9:35 AM on November 4, 2019: contributor

@hebasto - done. But can I make the point - by NOT fixing "historical" spelling issues in the release notes - we are also choosing to allow them and NEW ERRORS to persist into the future? What is the value of letting them persist? If we fix the old issues NOW we can enforce better policy and linting moving forward? Couldn't we even implement a build step that focuses solely on these types of issues or remove any rules that ignore documentation? (that was the thought behind editing the release notes to begin with)

Thanks! 👍

MarcoFalke referenced this in commit 73b26e38d7 on Nov 4, 2019

MarcoFalke merged this on Nov 4, 2019

MarcoFalke closed this on Nov 4, 2019

laanwj commented at 2:52 PM on November 4, 2019: member

But can I make the point - by NOT fixing "historical" spelling issues in the release notes - we are also choosing to allow them and NEW ERRORS to persist into the future?

They are historical documents, which have been published to many places besides this repository. Fixing errors there would be like going to a museum and scribbling in a 12th century bible, for example :smile: It's no reason to allow new errors in new files, just, what has been done has been done.

MarkLTZ referenced this in commit 8823f65197 on Apr 7, 2020

MarkLTZ referenced this in commit 0a1571d657 on Apr 7, 2020

jasonbcox referenced this in commit 6e032cad49 on Oct 30, 2020

MarcoFalke locked this on Dec 16, 2021