Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Get CI into production-ready shape #1

Merged
merged 6 commits into from
Dec 2, 2024
Merged

Get CI into production-ready shape #1

merged 6 commits into from
Dec 2, 2024

Conversation

DanGould
Copy link

@DanGould DanGould commented Dec 2, 2024 •
edited
Loading

  • Clippy lints
  • Use Rust MSRV 1.63.0

@DanGould DanGould closed this Dec 2, 2024
@DanGould DanGould reopened this Dec 2, 2024
@DanGould DanGould force-pushed the pin-1-56 branch 4 times, most recently from 7a803cb to 4a3a5b1 Compare December 2, 2024 16:14
@DanGould DanGould changed the title Pin cc for MSRV 1.56.1 Get CI into production-ready shape Dec 2, 2024
@DanGould DanGould requested a review from nothingmuch December 2, 2024 16:15
@DanGould DanGould mentioned this pull request Dec 2, 2024
Merged
@nothingmuch
Copy link

Can you explain the difference between 1.56 and 1.63 in the context of Kixunil#24 ? i'm not sure if the reason for bumping to 1.63 is simply that that's what rust-payjoin requires or if this is motivated by some code changes

@DanGould
Copy link
Author

DanGould commented Dec 2, 2024 •
edited
Loading

Can you explain the difference between 1.56 and 1.63 in the context of Kixunil#24 ? i'm not sure if the reason for bumping to 1.63 is simply that that's what rust-payjoin requires or if this is motivated by some code changes

  • rust-payjoin / rust-bitcoin now allows 1.63.0 MSRV since Pin cc for MSRV 1.56.1 Kixunil/bip21#24 was opened
  • Originally, I had to pin cc for 1.56.1 compat. The pin would break the build now because of a change in secp256k1-sys
  • Everything works without pinning if we can use 1.63.0 MSRV now

nothingmuch
nothingmuch approved these changes Dec 2, 2024
View reviewed changes
Copy link

@nothingmuch nothingmuch left a comment

Choose a reason for hiding this comment

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

This seems correct to me, so ACK, but I'm taking MSRV rationale at face value.

I would prefer silencing the remaining linting checks with an attr, so that it's easier to tell before CI if new warnings were introduced.

@DanGould
cargo clippy -all-targets --all-features --fix
71a8ae2
@DanGould
Move 'b lifetime to 'a since 'a was elided
a827cb3
@DanGould
Use rust MSRV 1.63.0
9242659
@DanGould
Act on: pull-request
63062b1
@DanGould
Silence unused UriErerorInner dead code
8cc5ff7 
The error is only passed up via `source` when `std::error::Error` is
available. When the feature is off, it's dead code.
@DanGould
Allow dead code for ParamBytes inner
2675825 
The code isn't actually dead, and is used in the bytes() function, but the
compiler's dead code analysis doesn't fully track usage through feature flags
and doesn't count if the pattern isn't considered a read of the field.
@DanGould DanGould mentioned this pull request Dec 2, 2024
Merged
@nothingmuch
Copy link

nothingmuch commented Dec 2, 2024 •
edited
Loading

post merge (edit: i saw purple in the mention just above and thought this was merged, sorry) utACK including dead code fixes

@DanGould DanGould merged commit 76db0d1 into payjoin:master Dec 2, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nothingmuch nothingmuch nothingmuch approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@DanGould @nothingmuch