Page MenuHomePhabricator

Merge #12716: Fix typos and cleanup in various files
Needs ReviewPublic

Authored by nakihito on Mon, Aug 5, 22:05.

Details

Reviewers
deadalnix
Fabien
jasonbcox
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Summary

4d9b4256d8 Fix typos (Dimitris Apostolou)

Pull request description:

Unfortunately I messed up my repo while trying to squash #12593 so I created a PR with just the correct fixes.

Tree-SHA512: 295d77b51bd2a9381f1802c263de7ffb2edd670d9647391e32f9a414705b3c8b483bb0e469a9b85ab6a70919ea13397fa8dfda2aea7a398b64b187f178fe6a06

Backport of Core PR12716
https://github.com/bitcoin/bitcoin/pull/12716/

Test Plan
make check
arc lint

Diff Detail

Repository
rABC Bitcoin ABC
Branch
PR12716
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 7086
Build 12218: Bitcoin ABC Teamcity Staging
Build 12217: arc lint + arc unit

Event Timeline

nakihito created this revision.Mon, Aug 5, 22:05
Owners added a reviewer: Restricted Owners Package.Mon, Aug 5, 22:05
Herald added a reviewer: Restricted Project. · View Herald TranscriptMon, Aug 5, 22:05
jasonbcox requested changes to this revision.Mon, Aug 5, 22:21
jasonbcox added inline comments.
src/qt/coincontroldialog.cpp
237 ↗(On Diff #10619)

its -> it is

408 ↗(On Diff #10619)

ditto

src/test/txvalidationcache_tests.cpp
38 ↗(On Diff #10619)

Add this one to the spellcheck linter

src/wallet/rpcwallet.cpp
2266 ↗(On Diff #10619)

fix multi-line string

test/functional/test_framework/mininode.py
493 ↗(On Diff #10619)

add this one to the linter too

This revision now requires changes to proceed.Mon, Aug 5, 22:21
nakihito updated this revision to Diff 10621.Tue, Aug 6, 01:03

Fixed its and muli-line string. Added new mispellings to dictionary.

nakihito added inline comments.Tue, Aug 6, 01:59
src/tinyformat.h
158 ↗(On Diff #10621)

The change to workaround was done manually. When the linter attempted to fix this typo, it would replace with Work Around rather than Work around. I do not know whether this is expected or unexpected behavior, but it would have been incorrect regardless.

nakihito edited the test plan for this revision. (Show Details)Tue, Aug 6, 02:00
deadalnix accepted this revision.Tue, Aug 6, 06:14
deadalnix requested changes to this revision.
deadalnix added inline comments.
test/lint/dictionary/english.json
634 ↗(On Diff #10621)

Workaround is commonly used as a noun: https://en.wikipedia.org/wiki/Workaround

This is not good.

This revision now requires changes to proceed.Tue, Aug 6, 06:16
Fabien added inline comments.Tue, Aug 6, 07:04
test/functional/test_framework/mininode.py
493 ↗(On Diff #10619)

You don't want to add this one to the linter, because all 3 words (work, around and workaround) are valid.
The linter has no way to get the context and will just replace a valid word by the others under every circumstance, which is not what you want.

I will look at another spell check, maybe in addition to this one, because there are a lot of leftovers and this cause a lot of maintenance.

nakihito updated this revision to Diff 10634.Tue, Aug 6, 16:40

Reverted workaround change.

jasonbcox requested changes to this revision.Tue, Aug 6, 17:20
jasonbcox added inline comments.
src/wallet/rpcwallet.cpp
2266 ↗(On Diff #10634)

???

This revision now requires changes to proceed.Tue, Aug 6, 17:20
nakihito updated this revision to Diff 10636.Tue, Aug 6, 17:30

Fixed typo.

Fabien requested changes to this revision.Wed, Aug 7, 07:37

The fix for guiutil.h is missing.
The fix for interpreter.cpp is missing (moved to sigencoding.cpp, IsValidDERSignatureEncoding).
The fix for util.h is missing (moved to logging.h).
The fix for test_framework.py is missing.

You can keep the change to workaround in mininode.py, just don't add it to the dictionary.

test/functional/wallet_abandonconflict.py
161 ↗(On Diff #10636)

While you're at it you can remove the extra spaces (or improve the layout).
This is not really out-of-scope as the original PR has a fix for this comment (that doesn't apply to us).

This revision now requires changes to proceed.Wed, Aug 7, 07:37
nakihito updated this revision to Diff 10656.Wed, Aug 7, 19:59

Addressed critiques.

nakihito planned changes to this revision.Wed, Aug 7, 19:59
nakihito requested review of this revision.Wed, Aug 7, 23:26
nakihito added inline comments.
src/tinyformat.h
158 ↗(On Diff #10656)

This workaround is being used as a noun as pointed out here: https://en.wikipedia.org/wiki/Workaround and is therefore okay.

test/functional/test_framework/mininode.py
493 ↗(On Diff #10656)

This workaround is not being used as a noun. Rather, it is being used to describe the method by which something is being done and as such should be split into two different words.

Fabien requested changes to this revision.Thu, Aug 8, 07:16
Fabien added inline comments.
test/functional/test_framework/mininode.py
513 ↗(On Diff #10656)

This one is still missing

test/functional/wallet_abandonconflict.py
161 ↗(On Diff #10656)

That's better, but you can remove one more. A single space is enough.

This revision now requires changes to proceed.Thu, Aug 8, 07:16
nakihito updated this revision to Diff 10681.Thu, Aug 8, 18:00

Fixed formatting and another typo.

Fabien accepted this revision.Thu, Aug 8, 18:29
jasonbcox accepted this revision.Thu, Aug 8, 19:44