Skip to content

Fix/ci update 1.56#454

Merged
notmandatory merged 3 commits intobitcoindevkit:masterfrom
afilini:fix/ci-update-1.56
Oct 26, 2021
Merged

Fix/ci update 1.56#454
notmandatory merged 3 commits intobitcoindevkit:masterfrom
afilini:fix/ci-update-1.56

Conversation

@afilini
Copy link
Copy Markdown
Member

@afilini afilini commented Oct 22, 2021

Description

Update our CI to use 1.56, pin some dependencies to maintain our MSRV.

Closes #439.

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

Bugfixes:

  • I'm linking the issue being fixed by this PR

@afilini afilini force-pushed the fix/ci-update-1.56 branch 4 times, most recently from 6963d40 to 3fb5d70 Compare October 22, 2021 12:47
@afilini afilini force-pushed the fix/ci-update-1.56 branch from 3fb5d70 to d75d221 Compare October 22, 2021 13:58
Copy link
Copy Markdown
Contributor

@rajarshimaitra rajarshimaitra left a comment

Choose a reason for hiding this comment

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

tACK

Are those refs changes required for the previous version update changes? Its working ok too without the refs changes.

.scan(checkpoint, |prev_header, ((_, filter_content), header)| {
let filter = BlockFilter::new(&filter_content);
if header != filter.filter_header(&prev_header) {
if header != filter.filter_header(prev_header) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are moving objects instead of passing refs more efficient for these cases for some reason?

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.

This was actually a clippy error, I'm not sure why the explicit & is not needed, but clearly the compiler can infer that it's a reference.

If you tried running clippy locally maybe you forgot to enable the compact_filters feature? This would explain why you didn't get anything

Copy link
Copy Markdown
Member

@notmandatory notmandatory left a comment

Choose a reason for hiding this comment

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

ACK d75d221

@notmandatory notmandatory merged commit d75d221 into bitcoindevkit:master Oct 26, 2021
@notmandatory notmandatory mentioned this pull request Oct 28, 2021
3 tasks
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.

Update CI pipelines to latest Rust stable 1.55.0

3 participants