Skip to content

Remove 1.29 checks from CI pipeline#964

Merged
apoelstra merged 2 commits intorust-bitcoin:masterfrom
tcharding:ci-msrv
Apr 22, 2022
Merged

Remove 1.29 checks from CI pipeline#964
apoelstra merged 2 commits intorust-bitcoin:masterfrom
tcharding:ci-msrv

Conversation

@tcharding
Copy link
Copy Markdown
Member

@tcharding tcharding commented Apr 21, 2022

We no longer need to test against Rust 1.29 now that v0.28 is out. In order to allow minor changes to be merged (i.e., not the big MSRV patchset) that rely on MSRV being 1.41.1 update the CI pipeline.

Remove the pinning stuff from contrib/test.sh while we are at it.

@tcharding
Copy link
Copy Markdown
Member Author

hmm, is there something else we need to do to disable the 1.29 job?

Copy link
Copy Markdown
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.

Other than the version issue this looks great.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I recommend using 1.41.1 since AFAIR 1.41.0 had miscompilation bugs.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

My bad, that is a mistake. Thanks for noticing.

@apoelstra
Copy link
Copy Markdown
Member

Yes, I need to go into the github UI to disable the 1.29 check. I'll do that as soon as this is merged.

+1 to changing to 1.41.1.

In preparation for starting to merge patches that rely on an MSRV of
1.41 update the CI pipeline.
We no longer need to test Rust 1.29, remove the pinning from the test
script.
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 8f45723.

I think we should re-visit all of our dependencies in Cargo.toml to see everything else that we can update. I can see serde_json = "<1.0.45", maybe we can have a later version now?

# We need to pin ryu (transitive dep from serde_json) to stay compatible with Rust 1.22.0
ryu = "<1.0.5"

To be clear, we should this in a separate PR afterward.

@tcharding
Copy link
Copy Markdown
Member Author

I think we should re-visit all of our dependencies

Good! I had not thought of that. Thanks

Copy link
Copy Markdown
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 8f45723

@apoelstra apoelstra merged commit d6f5d98 into rust-bitcoin:master Apr 22, 2022
@tcharding tcharding deleted the ci-msrv branch April 25, 2022 04:22
ChallengeDev210 pushed a commit to ChallengeDev210/rust-bitcoin that referenced this pull request Aug 1, 2022
8f45723 test.sh: Remove 1.29 pinning (Tobin C. Harding)
15bae28 Remove CI check for Rust 1.29 (Tobin C. Harding)

Pull request description:

  We no longer need to test against Rust 1.29 now that v0.28 is out. In order to allow minor changes to be merged (i.e., not the _big_ MSRV patchset) that rely on MSRV being 1.41.1 update the CI pipeline.

  Remove the pinning stuff from `contrib/test.sh` while we are at it.

ACKs for top commit:
  sanket1729:
    ACK 8f45723.
  Kixunil:
    ACK 8f45723

Tree-SHA512: acad2aa75077fd6959b85758aaa2265ae1f9c72a92edb528072cac68e83b903fcd2d15c9410ba35570132d927cf3d0bbc2deb537ba2298958476b0b8e5f2e149
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.

4 participants