docs: Improve netaddress comments#15718
Merged
laanwj merged 1 commit intobitcoin:masterfrom Apr 11, 2019
Merged
Conversation
Contributor
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
dongcarl
force-pushed
the
2019-04-netaddr-comments
branch
from
dba9b63 to
b11dbf7
Compare
dongcarl
changed the title
Contributor
Author
|
This is now ready for review. |
Contributor
|
Concept ACK -- nice clarification! Will review. A minor nit: make sure "IPv4", "IPv6" or "IP" are capitalised consistently. Some cases of "ip" in the current version. |
dongcarl
force-pushed
the
2019-04-netaddr-comments
branch
from
b11dbf7 to
7d3ee5c
Compare
Contributor
Author
|
Fixed some wording and changed some cases of "ip" to "address." |
jonatack
reviewed
Apr 4, 2019
dongcarl
force-pushed
the
2019-04-netaddr-comments
branch
from
7d3ee5c to
cf19e24
Compare
Contributor
Author
|
Addressed nits from @jonatack |
ryanofsky
approved these changes
Apr 5, 2019
dongcarl
force-pushed
the
2019-04-netaddr-comments
branch
from
cf19e24 to
fcf39fb
Compare
Contributor
Author
|
@ryanofsky Thanks for the detailed comments. Definitely clearer and better wordings. Fixed! |
Contributor
|
Please squash and this should be ready for merge :-) |
dongcarl
force-pushed
the
2019-04-netaddr-comments
branch
from
fcf39fb to
c99497c
Compare
Contributor
Author
|
Squashed. |
ryanofsky
approved these changes
Apr 9, 2019
Contributor
ryanofsky
left a comment
ryanofsky
left a comment
There was a problem hiding this comment.
utACK c99497c85f3576a4f4c8ede56074d81b59bfe57a. Just rebase, squash and a few suggested changes since last review. Since the PR is documentation only and the comments are easy to verify, this might be a good candidate for merge soon.
dongcarl
force-pushed
the
2019-04-netaddr-comments
branch
from
c99497c to
303372c
Compare
- Improve IsRFC methods docs
- Improve {Is,Set}Internal docs
- Add tor methods docs
- Add IsIPv{4,6} docs
- Add IsValid docs
- Add IsRoutable docs
- Improve GetGroup docs
- Add CService::GetSockAddr docs
- Add CService::GetKey docs
- Add CSubNet::Match docs
- Add NetmaskBits docs
- Add CNetAddr default constructor docs
Contributor
Author
|
Added doc for |
laanwj
added a commit
that referenced
this pull request
Apr 11, 2019
303372c docs: Improve netaddress comments (Carl Dong) Pull request description: Improves comments for `netaddress`, making them available to Doxygen. I think this is worthwhile because a lot of the code require some context (e.g., A lot of the things that we do to fit hostnames and tor addresses into `CNetAddr` is non-obvious, and documenting it is beneficial). ACKs for commit 303372: Tree-SHA512: 2a35784a01ed8ec5fdbe111a540192d31bde16afa96e4be97b0385daf290fc7469a66d7cb8905a70b920fad6a0e7400ca4e5da082d6e4af1d1aaccc0e8297720
laanwj
added a commit
that referenced
this pull request
Jun 6, 2019
8be3f30 netaddress: Update CNetAddr for ORCHIDv2 (Carl Dong) Pull request description: ``` The original ORCHID prefix was deprecated as of 2014-03, the new ORCHIDv2 prefix was allocated by RFC7343 as of 2014-07. We did not consider the original ORCHID prefix routable, and I don't see any reason to consider the new one to be either. ``` Would like to know if people think this kind of thing is even worth keeping the codebase updated for. Perhaps it'd be nice to write a devtool to pull the csv from [here](https://www.iana.org/assignments/iana-ipv6-special-registry/iana-ipv6-special-registry.xhtml) and generate the code. ACKs for commit 8be3f3: laanwj: utACK 8be3f30 ryanofsky: utACK 8be3f30. Only change since last review is rebasing after #15718 merge. Tree-SHA512: 7c93317f597b1a6c1443e12dd690010392edb9d72a479a8201970db7d3444fbb99a80b98026caad6fbfbebb455ab4035d2dde79bc9263bfd1d0398cd218392e1
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Jun 6, 2019
8be3f30 netaddress: Update CNetAddr for ORCHIDv2 (Carl Dong) Pull request description: ``` The original ORCHID prefix was deprecated as of 2014-03, the new ORCHIDv2 prefix was allocated by RFC7343 as of 2014-07. We did not consider the original ORCHID prefix routable, and I don't see any reason to consider the new one to be either. ``` Would like to know if people think this kind of thing is even worth keeping the codebase updated for. Perhaps it'd be nice to write a devtool to pull the csv from [here](https://www.iana.org/assignments/iana-ipv6-special-registry/iana-ipv6-special-registry.xhtml) and generate the code. ACKs for commit 8be3f3: laanwj: utACK 8be3f30 ryanofsky: utACK 8be3f30. Only change since last review is rebasing after bitcoin#15718 merge. Tree-SHA512: 7c93317f597b1a6c1443e12dd690010392edb9d72a479a8201970db7d3444fbb99a80b98026caad6fbfbebb455ab4035d2dde79bc9263bfd1d0398cd218392e1
deadalnix
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Jun 9, 2020
Summary: 303372c41a8d5c58a46cf9ed595e30e67bd0bc99 docs: Improve netaddress comments (Carl Dong) Pull request description: Improves comments for `netaddress`, making them available to Doxygen. I think this is worthwhile because a lot of the code require some context (e.g., A lot of the things that we do to fit hostnames and tor addresses into `CNetAddr` is non-obvious, and documenting it is beneficial). ACKs for commit 303372: Tree-SHA512: 2a35784a01ed8ec5fdbe111a540192d31bde16afa96e4be97b0385daf290fc7469a66d7cb8905a70b920fad6a0e7400ca4e5da082d6e4af1d1aaccc0e8297720 Backport of Core [[bitcoin/bitcoin#15718 | PR15718]] Test Plan: ninja Reviewers: O1 Bitcoin ABC, #bitcoin_abc, jasonbcox Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, jasonbcox Differential Revision: https://reviews.bitcoinabc.org/D6494
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jan 18, 2021
8be3f30 netaddress: Update CNetAddr for ORCHIDv2 (Carl Dong) Pull request description: ``` The original ORCHID prefix was deprecated as of 2014-03, the new ORCHIDv2 prefix was allocated by RFC7343 as of 2014-07. We did not consider the original ORCHID prefix routable, and I don't see any reason to consider the new one to be either. ``` Would like to know if people think this kind of thing is even worth keeping the codebase updated for. Perhaps it'd be nice to write a devtool to pull the csv from [here](https://www.iana.org/assignments/iana-ipv6-special-registry/iana-ipv6-special-registry.xhtml) and generate the code. ACKs for commit 8be3f3: laanwj: utACK 8be3f30 ryanofsky: utACK 8be3f30. Only change since last review is rebasing after bitcoin#15718 merge. Tree-SHA512: 7c93317f597b1a6c1443e12dd690010392edb9d72a479a8201970db7d3444fbb99a80b98026caad6fbfbebb455ab4035d2dde79bc9263bfd1d0398cd218392e1
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Improves comments for
netaddress, making them available to Doxygen.I think this is worthwhile because a lot of the code require some context (e.g., A lot of the things that we do to fit hostnames and tor addresses into
CNetAddris non-obvious, and documenting it is beneficial).