Skip to content

Capitalize proper nouns#2916

Merged
apoelstra merged 6 commits intorust-bitcoin:masterfrom
jamillambert:202400625-Capitalization
Jul 2, 2024
Merged

Capitalize proper nouns#2916
apoelstra merged 6 commits intorust-bitcoin:masterfrom
jamillambert:202400625-Capitalization

Conversation

@jamillambert
Copy link
Copy Markdown
Contributor

@jamillambert jamillambert commented Jun 25, 2024 •
edited
Loading

Taproot, Merkle and Huffman are proper nouns and should be capitalized in strings and docs.

These have been capitalized everywhere.

All cases of taproot/taptree/tap tree in docs and strings have been changed to be Taproot tree or [`TapTree`] as appropriate.

All cases of tree have been changed to lower cases, except where it is in title case.

Error messages in Taproot have been changed to start with a lowercase character, except when it is a proper noun, and not end with a period.

Close: #2913

@github-actions github-actions bot added the C-bitcoin PRs modifying the bitcoin crate label Jun 25, 2024
@coveralls
Copy link
Copy Markdown

coveralls commented Jun 25, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9660345835

Details

  • 5 of 18 (27.78%) changed or added relevant lines in 5 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 83.143%
Changes Missing Coverage Covered Lines Changed/Added Lines %
bitcoin/src/crypto/sighash.rs 0 1 0.0%
bitcoin/src/crypto/taproot.rs 0 1 0.0%
bitcoin/src/psbt/mod.rs 3 4 75.0%
bitcoin/src/psbt/error.rs 1 3 33.33%
bitcoin/src/taproot/mod.rs 1 9 11.11%
Totals Coverage Status
Change from base Build 9639464882: 0.0%
Covered Lines: 19542
Relevant Lines: 23504
💛 - Coveralls

@jamillambert jamillambert force-pushed the 202400625-Capitalization branch from 0f7671a to a93bc72 Compare June 25, 2024 10:00
@coveralls
Copy link
Copy Markdown

coveralls commented Jun 25, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9660391558

Details

  • 5 of 18 (27.78%) changed or added relevant lines in 5 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 83.143%
Changes Missing Coverage Covered Lines Changed/Added Lines %
bitcoin/src/crypto/sighash.rs 0 1 0.0%
bitcoin/src/crypto/taproot.rs 0 1 0.0%
bitcoin/src/psbt/mod.rs 3 4 75.0%
bitcoin/src/psbt/error.rs 1 3 33.33%
bitcoin/src/taproot/mod.rs 1 9 11.11%
Totals Coverage Status
Change from base Build 9639464882: 0.0%
Covered Lines: 19542
Relevant Lines: 23504
💛 - Coveralls

@tcharding
Copy link
Copy Markdown
Member

tcharding commented Jun 25, 2024

Except for the line length the commit log of fc40cf099cd90ab731addac2bb9b9914514bea60 is excellent, I knew exactly why the patch was needed and what it did - super easy to review.

tcharding
tcharding previously approved these changes Jun 25, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@tcharding tcharding left a comment

Choose a reason for hiding this comment

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

ACK a93bc72f10e4e303d1aaa18adba2fd2a0edfaf8f

@tcharding
Copy link
Copy Markdown
Member

tcharding commented Jun 25, 2024

Nice PR!

@jamillambert
Copy link
Copy Markdown
Contributor Author

jamillambert commented Jun 26, 2024

Except for the line length the commit log of fc40cf0 is excellent, I knew exactly why the patch was needed and what it did - super easy to review.

Thanks, and sorry. My editor had the setting switched off. I have now enabled a 50 char limit for the commit subject and 72 chars for the message.

@tcharding
Copy link
Copy Markdown
Member

tcharding commented Jun 26, 2024

Legend.

Kixunil
Kixunil previously approved these changes Jun 27, 2024
View reviewed changes
Copy link
Copy Markdown
Collaborator

@Kixunil Kixunil left a comment

Choose a reason for hiding this comment

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

ACK a93bc72f10e4e303d1aaa18adba2fd2a0edfaf8f

@jamillambert jamillambert dismissed stale reviews from Kixunil and tcharding via 7e45c73 June 28, 2024 07:14
@jamillambert jamillambert force-pushed the 202400625-Capitalization branch from a93bc72 to 7e45c73 Compare June 28, 2024 07:14
@github-actions github-actions bot added the doc label Jun 28, 2024
@coveralls
Copy link
Copy Markdown

coveralls commented Jun 28, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9708956202

Details

  • 15 of 39 (38.46%) changed or added relevant lines in 7 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage remained the same at 83.094%
Changes Missing Coverage Covered Lines Changed/Added Lines %
bitcoin/src/crypto/sighash.rs 0 1 0.0%
bitcoin/src/crypto/taproot.rs 0 1 0.0%
bitcoin/src/psbt/mod.rs 3 4 75.0%
bitcoin/src/merkle_tree/block.rs 5 7 71.43%
bitcoin/src/psbt/error.rs 1 3 33.33%
bitcoin/src/taproot/mod.rs 5 22 22.73%
Files with Coverage Reduction New Missed Lines %
bitcoin/src/taproot/mod.rs 1 78.13%
Totals Coverage Status
Change from base Build 9702831343: 0.0%
Covered Lines: 19508
Relevant Lines: 23477
💛 - Coveralls

@jamillambert
Copy link
Copy Markdown
Contributor Author

jamillambert commented Jun 28, 2024 •
edited
Loading

Added 3 patches to extend this pull request to fix everything in the corresponding issue.

Changed the commit messages to split long lines.

Changed the PR title from Capitalize Taproot to Capitalize proper nouns and updated the description.

@jamillambert jamillambert changed the title Capitalize Taproot Capitalize proper nouns Jun 28, 2024
@tcharding
Copy link
Copy Markdown
Member

tcharding commented Jun 28, 2024 •
edited
Loading

I love this PR, you are flying! I think the last patch should be removed and done as a separate PR.

EDIT: I deleted some stuff to save you reading it, I did some more research and I now think the last patch is correct (even if its slightly off topic for the PR)

tcharding
tcharding previously approved these changes Jun 28, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@tcharding tcharding left a comment

Choose a reason for hiding this comment

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

ACK 7e45c73f17ca0624dfdbc699ee749eb6e350fe3f

@apoelstra
Copy link
Copy Markdown
Member

apoelstra commented Jun 30, 2024

utACK 7e45c73f17ca0624dfdbc699ee749eb6e350fe3f. Thanks! Changes like this make the crate feel much more professional.

But this needs a rebase now, sorry.

@jamillambert jamillambert force-pushed the 202400625-Capitalization branch from 7e45c73 to aa5c654 Compare July 1, 2024 16:29
@jamillambert
Capitalize Taproot
c8e6c13 
Taproot is a proper noun and should be capitalized in docs and strings.

Make all occurrences of Taproot in comments or strings capitalized.
@jamillambert
Remove tap tree
b48ca05 
tap tree should be either Taproot tree in normal language or
[`TapTree`] when referring to the struct.

Change all occurances of tap tree or taptree.
@jamillambert
Do not capitalize tree
be5b93e 
Change tree to lower case except when title case is being used.
@jamillambert
Capitalize Merkle
175f69a 
Merkle is a proper noun and should be capitalized in docs and strings.

Capitalize all occurances of Merkle in docs and strings.
@jamillambert
Capitalize Huffman
717da4b 
Huffman is a proper noun and should be capitalized in docs and strings.

Capitalize all occurances of Huffman in docs and strings.
@jamillambert
taproot: Lowercase error messages
e8a30bf 
By convention error messages should not be capitalised or use full
stops.

Fix the `taproot` module error messages.
@jamillambert jamillambert force-pushed the 202400625-Capitalization branch from aa5c654 to e8a30bf Compare July 1, 2024 16:42
@coveralls
Copy link
Copy Markdown

coveralls commented Jul 1, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9747984695

Details

  • 15 of 35 (42.86%) changed or added relevant lines in 7 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage remained the same at 83.004%
Changes Missing Coverage Covered Lines Changed/Added Lines %
bitcoin/src/crypto/sighash.rs 0 1 0.0%
bitcoin/src/crypto/taproot.rs 0 1 0.0%
bitcoin/src/psbt/mod.rs 3 4 75.0%
bitcoin/src/merkle_tree/block.rs 5 7 71.43%
bitcoin/src/psbt/error.rs 1 3 33.33%
bitcoin/src/taproot/mod.rs 5 18 27.78%
Files with Coverage Reduction New Missed Lines %
bitcoin/src/taproot/mod.rs 1 76.99%
Totals Coverage Status
Change from base Build 9745227269: 0.0%
Covered Lines: 19510
Relevant Lines: 23505
💛 - Coveralls

@jamillambert
Copy link
Copy Markdown
Contributor Author

jamillambert commented Jul 1, 2024

Rebased to remove conflict.

tcharding
tcharding approved these changes Jul 1, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@tcharding tcharding left a comment

Choose a reason for hiding this comment

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

ACK e8a30bf

apoelstra
apoelstra approved these changes Jul 2, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK e8a30bf

@apoelstra apoelstra merged commit a302656 into rust-bitcoin:master Jul 2, 2024
@jamillambert jamillambert deleted the 202400625-Capitalization branch September 4, 2024 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@apoelstra apoelstra apoelstra approved these changes

@tcharding tcharding tcharding approved these changes

@Kixunil Kixunil Kixunil left review comments

Assignees

No one assigned

Labels

C-bitcoin PRs modifying the bitcoin crate doc

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Doc: Capitalization of proper nouns

5 participants

@jamillambert @coveralls @tcharding @apoelstra @Kixunil