Skip to content

Get CI into production-ready shape#1

Merged
DanGould merged 6 commits into
payjoin:masterfrom
DanGould:pin-1-56
Dec 2, 2024
Merged

Get CI into production-ready shape#1
DanGould merged 6 commits into
payjoin:masterfrom
DanGould:pin-1-56

Conversation

@DanGould
Copy link
Copy Markdown

@DanGould DanGould commented Dec 2, 2024

  • 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
@nothingmuch
Copy link
Copy Markdown

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
Copy Markdown
Author

DanGould commented Dec 2, 2024

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

Copy link
Copy Markdown

@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.

The error is only passed up via `source` when `std::error::Error` is
available. When the feature is off, it's dead code.
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.
@nothingmuch
Copy link
Copy Markdown

nothingmuch commented Dec 2, 2024

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants