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

lint: update list of spelling linter false positives, bump to codespell 2.0.0 #20817

Merged

Conversation

theStack
Copy link
Contributor

@theStack theStack commented Dec 31, 2020

This small patch updates the ignore list for the spelling linter script (which uses codespell), both removing false-positives that are not relevant anymore and adding new ones. As suggested by jonatack, whose last name is now also part of the list :). Also changed the linter script to not check the gitian keys file, as suggested by hebasto. The codespell version used is bumped to most recent version 2.0.0, which is more aware of some terms that were previously needed in the ignorelist for v1.17.1, see #20817 (comment).

Running spelling linter on master branch (repeated findings in the same file are removed to keep the output short):

$ ./test/lint/lint-spelling.sh
contrib/gitian-keys/keys.txt:16: Atack ==> Attack
doc/developer-notes.md:1284: inout ==> input, in out
doc/psbt.md:122: Asend ==> Ascend, as end
src/bench/verify_script.cpp:27: Keypair ==> Key pair
src/blockencodings.h:30: Unser ==> Under, unset, unsure, user
src/compressor.h:65: Unser ==> Under, unset, unsure, user
src/core_read.cpp:131: presense ==> presence
src/index/disktxpos.h:21: blockIn ==> blocking
src/net_processing.h:67: anounce ==> announce
src/netaddress.h:486: compatiblity ==> compatibility
src/primitives/transaction.h:35: nIn ==> inn, min, bin, nine
src/qt/bitcoinunits.cpp:101: nIn ==> inn, min, bin, nine
src/rpc/blockchain.cpp:2150: nIn ==> inn, min, bin, nine
src/rpc/misc.cpp:198: nIn ==> inn, min, bin, nine
src/script/bitcoinconsensus.cpp:81: nIn ==> inn, min, bin, nine
src/script/bitcoinconsensus.h:63: nIn ==> inn, min, bin, nine
src/script/interpreter.cpp:1279: nIn ==> inn, min, bin, nine
src/script/interpreter.h:222: nIn ==> inn, min, bin, nine
src/script/sign.cpp:17: nIn ==> inn, min, bin, nine
src/script/sign.h:39: nIn ==> inn, min, bin, nine
src/serialize.h:181: Unser ==> Under, unset, unsure, user
src/signet.cpp:142: nIn ==> inn, min, bin, nine
src/test/base32_tests.cpp:17: fo ==> of, for
src/test/base64_tests.cpp:17: fo ==> of, for
src/test/script_tests.cpp:1509: nIn ==> inn, min, bin, nine
src/test/sighash_tests.cpp:27: nIn ==> inn, min, bin, nine
src/test/validation_tests.cpp:78: excercise ==> exercise
src/undo.h:36: Unser ==> Under, unset, unsure, user
src/validation.cpp:1403: nIn ==> inn, min, bin, nine
src/validation.h:255: nIn ==> inn, min, bin, nine
src/wallet/wallet.cpp:1532: nIn ==> inn, min, bin, nine
src/wallet/walletdb.cpp:429: Crypted ==> Encrypted
test/functional/feature_nulldummy.py:63: unnecssary ==> unnecessary
test/functional/wallet_encryption.py:81: crypted ==> encrypted
test/functional/wallet_upgradewallet.py:36: fpr ==> for, far, fps
^ 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

Running spelling linter on PR branch:

$ ./test/lint/lint-spelling.sh
src/core_read.cpp:131: presense ==> presence
src/net_processing.h:67: anounce ==> announce
src/netaddress.h:486: compatiblity ==> compatibility
src/test/validation_tests.cpp:78: excercise ==> exercise
src/wallet/walletdb.cpp:429: Crypted ==> Encrypted
test/functional/feature_nulldummy.py:63: unnecssary ==> unnecessary
test/functional/wallet_encryption.py:81: crypted ==> encrypted
^ 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

This list of remaining findings doesn't contain false positives anymore -- the typos are fixed in PR #20762.
Happy new year! 🍾

@theStack theStack mentioned this pull request Dec 31, 2020
@DrahtBot DrahtBot added the Tests label Dec 31, 2020
Copy link
Member

@jonatack jonatack left a comment

Choose a reason for hiding this comment

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

ACK 8b4e84d

$ test/lint/lint-spelling.sh 
src/core_read.cpp:131: presense ==> presence
src/net_processing.h:67: anounce ==> announce
src/netaddress.h:486: compatiblity ==> compatibility
src/test/validation_tests.cpp:78: excercise ==> exercise
src/wallet/walletdb.cpp:429: Crypted ==> Encrypted
test/functional/feature_nulldummy.py:63: unnecssary ==> unnecessary
test/functional/wallet_encryption.py:81: crypted ==> encrypted

Well done, thanks.

@practicalswift
Copy link
Contributor

ACK 8b4e84d

Happy new year! 🍾

Happy new year @theStack!

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

Concept ACK.

test/lint/lint-spelling.ignore-words.txt Outdated Show resolved Hide resolved
@theStack
Copy link
Contributor Author

theStack commented Jan 1, 2021

Added a commit to ignore the gitian keys file for linting, as suggested by hebasto, and updated the PR description accordingly.

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

Tested a63b712:

$ codespell --version
1.17.1
$ test/lint/lint-spelling.sh 
ci/lint/06_script.sh:29: keyserver ==> key server
contrib/macdeploy/gen-sdk:60: files' ==> file's
contrib/seeds/makeseeds.py:108: dedup ==> dedupe
contrib/seeds/makeseeds.py:189: dedup ==> dedupe
contrib/verify-commits/README.md:43: keyserver ==> key server
src/Makefile.am:278: OBJEXT ==> OBJECT
src/core_read.cpp:131: presense ==> presence
src/cuckoocache.h:152: copyable ==> copiable
src/net_processing.h:67: anounce ==> announce
src/netaddress.h:486: compatiblity ==> compatibility
src/support/lockedpool.h:55: copyable ==> copiable
src/support/lockedpool.h:166: copyable ==> copiable
src/test/validation_tests.cpp:78: excercise ==> exercise
src/wallet/walletdb.cpp:429: Crypted ==> Encrypted
src/zmq/zmqpublishnotifier.cpp:231: Dedup ==> Dedupe
test/functional/feature_nulldummy.py:63: unnecssary ==> unnecessary
test/functional/wallet_encryption.py:81: crypted ==> encrypted
^ 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

This list of remaining findings doesn't contain false positives anymore -- the typos are fixed in PR #20762.

Not sure about that statement.

From the Cirrus linter job log:

ci/lint/06_script.sh:29: keyserver ==> key server
contrib/macdeploy/gen-sdk:60: files' ==> file's
contrib/seeds/makeseeds.py:108: dedup ==> dedupe
contrib/seeds/makeseeds.py:189: dedup ==> dedupe
contrib/verify-commits/README.md:43: keyserver ==> key server
src/Makefile.am:278: OBJEXT ==> OBJECT
src/core_read.cpp:131: presense ==> presence
src/cuckoocache.h:152: copyable ==> copiable
src/net_processing.h:67: anounce ==> announce
src/netaddress.h:486: compatiblity ==> compatibility
src/support/lockedpool.h:55: copyable ==> copiable
src/support/lockedpool.h:166: copyable ==> copiable
src/test/validation_tests.cpp:78: excercise ==> exercise
src/wallet/walletdb.cpp:429: Crypted ==> Encrypted
src/zmq/zmqpublishnotifier.cpp:231: Dedup ==> Dedupe
test/functional/feature_nulldummy.py:63: unnecssary ==> unnecessary
test/functional/wallet_encryption.py:81: crypted ==> encrypted
^ 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

@DrahtBot
Copy link
Contributor

DrahtBot commented Jan 2, 2021

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

Conflicts

No conflicts as of last run.

@theStack
Copy link
Contributor Author

theStack commented Jan 2, 2021

@hebasto: Thanks for testing! I wasn't aware that I used already codespell 2.0.0 (recently released in 23 Nov 2020), which is obviously more aware of various terms like e.g. objext as Makefile term and copyable as C++ term (see their commit codespell-project/codespell@45b67b4 which fixed this).
Could either bump the codespell version from 1.17.1 to 2.0.0 in an additional (first) commit or simply restore the removed exceptions for objext, copyable, keyserver (and add dedup) from the false positives wordlist change.

@hebasto
Copy link
Member

hebasto commented Jan 2, 2021

Could either bump the codespell version from 1.17.1 to 2.0.0 in an additional (first) commit or simply restore the removed exceptions for objext, copyable, keyserver (and add dedup) from the false positives wordlist change.

I think the former is fine. The only concern is about codespell-project/codespell#1774.

@theStack theStack changed the title lint: update list of spelling linter false positives lint: update list of spelling linter false positives, bump to codespell 2.0.0 Jan 2, 2021
@theStack theStack force-pushed the 201212-lint-update-spelling-false-positives branch from a63b712 to b86ddfe Compare January 2, 2021 11:19
@theStack theStack force-pushed the 201212-lint-update-spelling-false-positives branch from b86ddfe to 852cd43 Compare January 2, 2021 11:21
Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK 852cd43, tested on Linux Mint 20 (x86_64).

test/lint/lint-spelling.ignore-words.txt Outdated Show resolved Hide resolved
test/lint/lint-spelling.ignore-words.txt Outdated Show resolved Hide resolved
theStack and others added 2 commits January 2, 2021 19:06
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
@theStack theStack force-pushed the 201212-lint-update-spelling-false-positives branch from 852cd43 to f3ba916 Compare January 2, 2021 18:08
@theStack
Copy link
Contributor Author

theStack commented Jan 2, 2021

Force-pushed, addressed @hebasto's suggestions of keeping the ignore-list in order (#20817 (comment)) and removing the "keypair" exception (#20817 (comment)).

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

re-ACK f3ba916, only suggested changes since my previous review.

Copy link
Member

@jonatack jonatack left a comment

Choose a reason for hiding this comment

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

ACK f3ba916 I don't know if there are any particular issues with bumping codespell to v2.0.0, but locally running the spelling linter and the cirrus job at https://cirrus-ci.com/task/5004066998714368 both LGTM. Thanks for also verifying and removing the unused words from the ignore list.

@fanquake fanquake merged commit f52f427 into bitcoin:master Jan 4, 2021
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jan 4, 2021
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants