8000 Add docs re Rust 1.51.1 by tcharding · Pull Request #1122 · rust-bitcoin/rust-bitcoin · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add docs re Rust 1.51.1 #1122

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 25, 2022
Merged

Conversation

tcharding
Copy link
Member

After auditing the code base for 'msrv' the only remaining mention (excl. md files) is on unsigned_abs.

Add docs to save the next guy looking up what version of Rust introduced unsigned_abs. (I also added an item to the msrv tracking issue.)

dpc
dpc previously approved these changes Jul 22, 2022 10000
@@ -336,7 +336,7 @@ fn dec_width(mut num: u64) -> usize {
width
}

// NIH due to MSRV, impl copied from `core`
// NIH due to MSRV, impl copied from `core::unsigned_abs` (Rust 1.51.1).
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you want to avoid confusion of people who don't know the method at all prefer the actual full path :) core::i8::unsigned_abs

Copy link
Member

Choose a reason for hiding this comment

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

If you're going to edit the line anyway, I'd also like it to say "introduced in Rust 1.51.1". As written it kinda looks like we copied the code from Rust 1.51.1, which might be good to know but doesn't seem particularly useful :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep and yep, sure thing. Thanks

Copy link
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 7f498ee

@Kixunil Kixunil added documentation 8000 code quality Makes code easier to understand and less likely to lead to problems trivial Obvious, easy and quick to review (few lines or doc-only...) one ack PRs that have one ACK, so one more can progress them no release notes mention labels Jul 24, 2022
Copy link
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 7f498ee

@apoelstra apoelstra merged commit 7953764 into rust-bitcoin:master Jul 25, 2022
@tcharding tcharding deleted the 07-22-msrv-doc branch August 1, 2022 05:11
ChallengeDev210 pushed a commit to ChallengeDev210/rust-bitcoin that referenced this pull request Aug 1, 2022
7f498ee Add docs re Rust 1.51.1 (Tobin C. Harding)

Pull request description:

  After auditing the code base for 'msrv' the only remaining mention (excl. md files) is on `unsigned_abs`.

  Add docs to save the next guy looking up what version of Rust introduced `unsigned_abs`. (I also added an item to the [msrv tracking issue](rust-bitcoin/rust-bitcoin#1060).)

ACKs for top commit:
  Kixunil:
    ACK 7f498ee
  apoelstra:
    ACK 7f498ee

Tree-SHA512: 9b4bdca09d3f7ac1c0584f4eb5207983a064cfe81ed26952d00396b2e0019ef40eee192b7f09d36c68b8c4e1f8de9cac2d1f3ee0946626bae089b46fe38704ab
@Kixunil Kixunil added this to the 0.29.0 milestone Aug 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code quality Makes code easier to understand and less likely to lead to problems no release notes mention one ack PRs that have one ACK, so one more can progress them trivial Obvious, easy and quick to review (few lines or doc-only...)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0