Fix typos #28605

Sjors · 2023-10-06T07:48:58Z

This PR fixes typos found by lint-spelling.py using codespell 2.2.6.

Our CI linter job uses codespell 2.2.5 and found fewer typos that I did locally. In any case it's happy now.

DrahtBot · 2023-10-06T07:49:01Z

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

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	pablomartin4btc
Concept NACK	fanquake, maflcko
Stale ACK	hebasto, jarolrod, hernanmarino, achow101, josibake

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#28710 (Remove the legacy wallet and BDB dependency by achow101)
#27788 (rpc: Be able to access RPC parameters by name by achow101)
#25273 (wallet: Pass through transaction locktime and preset input sequences and scripts to CreateTransaction by achow101)

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.

Sjors · 2023-10-06T07:50:51Z

test/lint/spelling.ignore-words.txt

@@ -6,6 +6,7 @@ bu
 cachable
 clen
 crypted
+debbugs


This is a reference to a Github repo in the Guix install doc.

src/minisketch/src/sketch_impl.h

Sjors · 2023-10-06T08:00:39Z

Updated to also fix re-use -> reuse (except in historical release notes, which the linter skips).

src/test/fuzz/FuzzedDataProvider.h

Sjors · 2023-10-06T08:09:21Z

Dropped typo fix in sketch_impl.h and added a warning to the release process to not (accidentally) touch typos in upstream code.

hernanmarino · 2023-10-06T22:13:42Z

This is a nice idea. I always have a conflict between submitting this simple typo-fixing PRs and ignoring them. This should be done often, for evey release .

We probably don't want too many pull requests that just fix a typo. At the same time, if we never fix them people will start ignore the linter all-together. So my suggestion would be to do this before branch-off.

hebasto · 2023-10-07T11:42:51Z

Our CI linter job uses codespell 2.2.5 and found fewer typos that I did locally. In any case it's happy now.

Why not bumping the codespell version in CI simultaneously?

hebasto

I reckon that changes in such a pull request should be forced by a linter. However, with the picked change into the spelling.ignore-words.txt file, the linter output is as follows:

$ ./test/lint/lint-spelling.py 
src/addrman.h:175: occurence ==> occurrence
src/net_processing.cpp:2450: annnounced ==> announced
src/net_processing.cpp:3595: succesful ==> successful
src/policy/policy.h:101: compatability ==> compatibility
src/qt/transactiontablemodel.cpp:222: re-use ==> reuse
src/rpc/server.cpp:412: unspecifed ==> unspecified
src/rpc/util.cpp:1091: re-use ==> reuse
src/rpc/util.h:409: documention ==> documentation
src/test/fuzz/FuzzedDataProvider.h:161: capaticity ==> capacity
src/test/orphanage_tests.cpp:114: Re-use ==> Reuse
src/test/script_tests.cpp:1115: re-use ==> reuse
src/torcontrol.cpp:254: implementors ==> implementers
src/txmempool.h:480: appplies ==> applies
src/wallet/feebumper.cpp:173: re-use ==> reuse
src/wallet/spend.cpp:1260: re-use ==> reuse
src/wallet/spend.cpp:1302: Re-use ==> Reuse
test/functional/test_framework/blockfilter.py:32: relvant ==> relevant
test/functional/wallet_avoidreuse.py:260: re-use ==> reuse
^ Warning: codespell identified likely spelling errors. Any false positives? Add them to the list of ignored words in test/lint/spelling.ignore-words.txt

Apparently, the linter ignores the word "re-used" in the following files:

ci/README.md
depends/description.md
src/test/versionbits_tests.cpp
src/wallet/wallet.h

The overall changes look consistent though.

ACK 650e22d.

Sjors · 2023-10-08T10:50:54Z

Why not bumping the codespell version in CI simultaneously?

I'll look into that if I have to retouch. At least we know that such a bump won't reveal new typos, so it can be done safely anytime.

jarolrod

ACK 650e22d

hernanmarino

ACK 650e22d

pablomartin4btc

ACK 650e22d

pablomartin4btc · 2023-10-16T14:49:22Z

src/torcontrol.cpp

-             *   For future-proofing, controller implementors MAY use the following
+             *   For future-proofing, controller implementers MAY use the following


Actually, both implementors & implementers are valid words, but it seems they have different meaning (Implementer: One who has implemented, Implementor: One who provides implementation).

achow101 · 2023-10-16T17:48:18Z

ACK 650e22d

fanquake · 2023-10-16T17:50:04Z

NACK on adding this to the release process. I don't understand why the CI infra isn't being changed here. This is the wrong solution to a non-problem.

fanquake · 2023-10-17T13:29:54Z

To clarify:

If the lint infra doesn't work (is ignored by everyone, and doesn't enforce the linting via failure) then it should be just removed. Otherwise, why do we have it? It basically just results in random PRs to "fix"/bump things.
If we are going to keep the infra, and change anything here, why wouldn't it be updated to the newest version available, and all issues fixed?
Adding anything like this to the release process is just reinforcement of the lint infra being pointless:
- You are just delegating to someone (probably a maintainer) to run codespell pre branch off.
- Trivial things should not be added to an (already messy) doc that contains important steps for creating a release.

so it can be done safely anytime.

We aren't changing consensus code here, why would it be unsafe to fix typos at any time?

josibake · 2023-10-17T14:09:31Z

Also a NACK on adding this to the release process. We should be looking for ways to reduce manual steps and further automate the release process. The last commit adds a manual step and doesn't address the root cause of how these spelling errors are getting merged in the first place.

I'd suggest we drop 650e22d and instead, look into updating the linting infrastructure so that these errors are caught going forward. If after fixing the linting infrastructure we still need manual fix up PRs, they should happen whenever someone is sufficiently motivated to do it, and not be tied to a release process. Ideally, any future fix-up PRs should also include updates to the linting infrastructure to prevent the same type of errors from sneaking through.

josibake · 2023-10-17T14:10:37Z

ACK e062212

maflcko · 2023-11-06T09:34:41Z

Are you still working on this? Otherwise, I think it can be closed, so that it can be picked up by someone else, if it is still relevant.

Sjors · 2023-11-06T09:43:18Z

I was still waiting for feedback from maintainers, but I'll just drop the last commit.

fanquake · 2023-11-06T09:56:34Z

test/functional/feature_assumeutxo.py:78: refering ==> referring
test/functional/feature_assumeutxo.py:115: refering ==> referring
^ Warning: codespell identified likely spelling errors. Any false positives? Add them to the list of ignored words in test/lint/spelling.ignore-words.txt

maflcko · 2023-11-06T10:17:34Z

I was still waiting for feedback from maintainers

DrahtBot has a summary comment, which has links to all feedback, including that of maintainers, see #28605 (comment)

As found by lint-spelling.py using codespell 2.2.6.

Sjors · 2023-11-07T01:22:26Z

Rebased to fix those two new typos.

pablomartin4btc

re ACK 43de4d3

fanquake · 2023-11-16T10:35:24Z

There are even more since this was last updated, but going to merge this now, and we can follow up again later.

Sjors commented Oct 6, 2023

View reviewed changes

maflcko reviewed Oct 6, 2023

View reviewed changes

src/minisketch/src/sketch_impl.h Outdated Show resolved Hide resolved

Sjors force-pushed the 2023/10/typos branch from b0ed6e5 to ad08990 Compare October 6, 2023 08:00

DrahtBot added the CI failed label Oct 6, 2023

maflcko reviewed Oct 6, 2023

View reviewed changes

src/test/fuzz/FuzzedDataProvider.h Show resolved Hide resolved

Sjors force-pushed the 2023/10/typos branch from ad08990 to 650e22d Compare October 6, 2023 08:08

DrahtBot removed the CI failed label Oct 6, 2023

hebasto approved these changes Oct 7, 2023

View reviewed changes

hebasto mentioned this pull request Oct 10, 2023

docs: fix typo #28624

Merged

DrahtBot mentioned this pull request Oct 11, 2023

p2p: peer connection bug fixes #28248

Closed

jarolrod reviewed Oct 12, 2023

View reviewed changes

DrahtBot requested review from hebasto and removed request for hebasto October 12, 2023 04:24

hernanmarino approved these changes Oct 16, 2023

View reviewed changes

pablomartin4btc reviewed Oct 16, 2023

View reviewed changes

DrahtBot removed the CI failed label Oct 27, 2023

Sjors changed the title ~~Fix typos and suggest doing so before branch-off~~ Fix typos Nov 6, 2023

Sjors force-pushed the 2023/10/typos branch from 650e22d to e062212 Compare November 6, 2023 09:44

DrahtBot requested review from hebasto, hernanmarino, pablomartin4btc, achow101 and jarolrod and removed request for josibake November 6, 2023 09:44

DrahtBot requested review from hernanmarino and removed request for hernanmarino November 6, 2023 09:56

DrahtBot requested review from hernanmarino and removed request for hernanmarino November 6, 2023 10:17

doc: fix typos

43de4d3

As found by lint-spelling.py using codespell 2.2.6.

Sjors force-pushed the 2023/10/typos branch from e062212 to 43de4d3 Compare November 7, 2023 01:22

pablomartin4btc approved these changes Nov 7, 2023

View reviewed changes

DrahtBot requested review from josibake and hernanmarino and removed request for hernanmarino November 7, 2023 01:46

DrahtBot requested review from hernanmarino and removed request for hernanmarino November 16, 2023 10:35

fanquake merged commit 22025d0 into bitcoin:master Nov 16, 2023

Sjors deleted the 2023/10/typos branch November 17, 2023 13:29

bitcoin locked and limited conversation to collaborators Nov 19, 2024

		* For future-proofing, controller implementors MAY use the following
		* For future-proofing, controller implementers MAY use the following

Fix typos #28605

Fix typos #28605

Uh oh!

Conversation

Sjors commented Oct 6, 2023 • edited by fanquake Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Oct 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage

Reviews

Conflicts

Uh oh!

Sjors Oct 6, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Sjors commented Oct 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Sjors commented Oct 6, 2023

Uh oh!

hernanmarino commented Oct 6, 2023

Uh oh!

hebasto commented Oct 7, 2023

Uh oh!

hebasto left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Sjors commented Oct 8, 2023

Uh oh!

jarolrod left a comment

Choose a reason for hiding this comment

Uh oh!

hernanmarino left a comment

Choose a reason for hiding this comment

Uh oh!

pablomartin4btc left a comment

Choose a reason for hiding this comment

Uh oh!

pablomartin4btc Oct 16, 2023

Choose a reason for hiding this comment

Uh oh!

achow101 commented Oct 16, 2023

Uh oh!

fanquake commented Oct 16, 2023

Uh oh!

fanquake commented Oct 17, 2023

Uh oh!

josibake commented Oct 17, 2023

Uh oh!

josibake commented Oct 17, 2023

Uh oh!

maflcko commented Nov 6, 2023

Uh oh!

Sjors commented Nov 6, 2023

Uh oh!

fanquake commented Nov 6, 2023

Uh oh!

maflcko commented Nov 6, 2023

Uh oh!

Sjors commented Nov 7, 2023

Uh oh!

pablomartin4btc left a comment

Choose a reason for hiding this comment

Uh oh!

fanquake commented Nov 16, 2023

Uh oh!

Uh oh!

Sjors commented Oct 6, 2023 •

edited by fanquake

Loading

DrahtBot commented Oct 6, 2023 •

edited

Loading

Sjors commented Oct 6, 2023 •

edited

Loading

hebasto left a comment •

edited

Loading