Skip to content

Clean up module level rustdocs#689

Merged
Kixunil merged 2 commits intorust-bitcoin:masterfrom
tcharding:module-rustdocs
Nov 16, 2021
Merged

Clean up module level rustdocs#689
Kixunil merged 2 commits intorust-bitcoin:masterfrom
tcharding:module-rustdocs

Conversation

@tcharding
Copy link
Copy Markdown
Member

@tcharding tcharding commented Nov 6, 2021

Docs can always do with a bit of love.

Clean up the module level (//!) rustdocs for all public modules.

I claim uniform is better than any specific method/style. I tried to fit
in with what ever was either most sane of most prevalent, therefore
attaining uniformity without unnecessary code churn (one exception being
the changes to headings described below).

Notes:

  • Headings - use heading as a regular sentence for all modules e.g.,
//! Bitcoin network messages.

as opposed to

//! # Bitcoin Network Messages

It was not clear which style to use so I picked a 'random' mature
project and copied their style.

  • Added 'This module' in most places as the start of the module
    description, however I was not religious about this one.

  • Fixed line length if necessary since most of our code seems to follow
    short (80 char) line lengths for comments anyways.

  • Added periods and fixed obvious (and sometimes not so obvious)
    grammatically errors.

  • Added a trailing //! to every block since this was almost universal
    already. I don't really like this one but I'm guessing it is Andrew's
    preferred style since its on the copyright notices as well.

@tcharding
Clean up module level rustdocs
3f5caa5 
Docs can always do with a bit of love.

Clean up the module level (`//!`) rustdocs for all public modules.

I claim uniform is better than any specific method/style. I tried to fit
in with what ever was either most sane of most prevalent, therefore
attaining uniformity without unnecessary code churn (one exception being
the changes to headings described below).

Notes:

* Headings - use heading as a regular sentence for all modules e.g.,

```
//! Bitcoin network messages.
```

as opposed to
```
//! # Bitcoin Network Messages
```

It was not clear which style to use so I picked a 'random' mature
project and copied their style.

* Added 'This module' in _most_ places as the start of the module
description, however I was not religious about this one.

* Fixed line length if necessary since most of our code seems to follow
short (80 char) line lengths for comments anyways.

* Added periods and fixed obvious (and sometimes not so obvious)
grammatically errors.

* Added a trailing `//!` to every block since this was almost universal
already. I don't really like this one but I'm guessing it is Andrew's
preferred style since its on the copyright notices as well.
dr-orlovsky
dr-orlovsky previously approved these changes Nov 12, 2021
View reviewed changes
Copy link
Copy Markdown
Collaborator

@dr-orlovsky dr-orlovsky left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thank you for the attention to details!

ACK 3f5caa5

BTW I always thought that we have 100-line comments here

src/util/contracthash.rs
//

//! Pay-to-contract-hash support
//! Pay-to-contract-hash support.
Copy link
Copy Markdown
Collaborator

@dr-orlovsky dr-orlovsky Nov 12, 2021

Choose a reason for hiding this comment

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

This module is kind of deprecated, so maybe it is worth mentioning it in the docs

Copy link
Copy Markdown
Member Author

@tcharding tcharding Nov 16, 2021

Choose a reason for hiding this comment

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

Added a comment to the rustdocs of contracthash module. Done as a separate patch.

sanket1729
sanket1729 previously approved these changes Nov 16, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@sanket1729 sanket1729 left a comment

Choose a reason for hiding this comment

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

ACK 3f5caa5. Thank you so much for doing this

@tcharding
Copy link
Copy Markdown
Member Author

tcharding commented Nov 16, 2021 •
edited
Loading

BTW I always thought that we have 100-line comments here

Thanks for letting me know, I'll go back over this PR with my editor set to 100 column-width.

EDIT: Point noted, I looked over this PR, I think I'll leave it as is because in this instance the shorter line length only adds a few lines of code and changing to 100 made these few blocks of docs, in my opinion, stand out even more. With rust-fmt on the horizon I think its ok to not be too strict on this right now. Holla at me though if you want them changed. Here is the diff that could be added to this PR if wanted.

diff --git a/src/hash_types.rs b/src/hash_types.rs
index 253d993..aeeb3c0 100644
--- a/src/hash_types.rs
+++ b/src/hash_types.rs
@@ -14,10 +14,9 @@

//! Bitcoin hash types.
//!
-//! This module defines types for hashes used throughout the library. These
-//! types are needed in order to avoid mixing data of the same hash format
-//! (e.g. `SHA256d`) but of different meaning (such as transaction id, block
-//! hash).
+//! This module defines types for hashes used throughout the library. These types are needed in
+//! order to avoid mixing data of the same hash format (e.g. `SHA256d`) but of different meaning
+//! (such as transaction id, block hash).
//!

use hashes::{Hash, sha256, sha256d, hash160};
diff --git a/src/util/bip158.rs b/src/util/bip158.rs
index 762c4de..41528e2 100644
--- a/src/util/bip158.rs
+++ b/src/util/bip158.rs
@@ -18,10 +18,9 @@

//! BIP158 Compact Block Filters for light clients.
//!
-//! This module implements a structure for compact filters on block data, for
-//! use in the BIP 157 light client protocol. The filter construction proposed
-//! is an alternative to Bloom filters, as used in BIP 37, that minimizes filter
-//! size by using Golomb-Rice coding for compression.
+//! This module implements a structure for compact filters on block data, for use in the BIP 157
+//! light client protocol. The filter construction proposed is an alternative to Bloom filters, as
+//! used in BIP 37, that minimizes filter size by using Golomb-Rice coding for compression.
//!
//! ## Example
//!

@tcharding
Add deprecation comment
dbb3edd 
Module `contracthash` is deprecated, add this info to the module
rustdoc.
@tcharding tcharding dismissed stale reviews from sanket1729 and dr-orlovsky via dbb3edd November 16, 2021 02:02
dr-orlovsky
dr-orlovsky approved these changes Nov 16, 2021
View reviewed changes
Copy link
Copy Markdown
Collaborator

@dr-orlovsky dr-orlovsky left a comment

Choose a reason for hiding this comment

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

ACK dbb3edd

@Kixunil Kixunil merged commit ab97d2d into rust-bitcoin:master Nov 16, 2021
@dr-orlovsky dr-orlovsky mentioned this pull request Jan 15, 2022
Closed
@tcharding tcharding deleted the module-rustdocs branch August 17, 2023 02:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Kixunil Kixunil Kixunil approved these changes

@sanket1729 sanket1729 sanket1729 left review comments

+1 more reviewer

@dr-orlovsky dr-orlovsky dr-orlovsky approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@tcharding @dr-orlovsky @Kixunil @sanket1729