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

bump version to 0.29.2, fix test_data exclude #1316

Merged
merged 5 commits into from Oct 26, 2022
Merged

bump version to 0.29.2, fix test_data exclude #1316

merged 5 commits into from Oct 26, 2022

Conversation

RCasatta
Copy link
Collaborator

@RCasatta RCasatta commented Oct 11, 2022 •
edited

backport of #1315

reduce crate package from 1.7M to 500k

@apoelstra
Copy link
Member

Looks like a new clippy lint is breaking CI

@RCasatta
Copy link
Collaborator Author

Should I fix clippy even if it's a backport?

sanket1729
sanket1729 approved these changes Oct 13, 2022
View reviewed changes
Copy link
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 708bf12

@Kixunil
Copy link
Collaborator

Kixunil commented Oct 14, 2022

Maybe we should bundle it together with a fix for #1307?

@apoelstra
Copy link
Member

@RCasatta yeah, we might as well fix it even in a backport, since it's quick and easy.

@Kixunil yes, agreed, let's bundle those together.

@RCasatta
Copy link
Collaborator Author

Clippy fixed

@sanket1729
Copy link
Member

Is the CI broken again :(

@apoelstra
Copy link
Member

I think this is an old break -- turning off features in honggfuzz -- that we also need to backport.

@RCasatta RCasatta force-pushed the fix_test_data_backport branch 2 times, most recently from 75c55aa to b11a7d0 Compare October 24, 2022 08:33
tcharding
tcharding approved these changes Oct 24, 2022
View reviewed changes
Copy link
Member

@tcharding tcharding left a comment

Choose a reason for hiding this comment

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

Patch 2: 8e2f93a could have been backported using #1299 but I confirmed that the diff of the two patches is identical.

ACK b11a7d0

@Kixunil
Copy link
Collaborator

Kixunil commented Oct 24, 2022

Shouldn't the first commit be the last one?

@apoelstra
Copy link
Member

I think it's fine -- both commits compile independently. There isn't really any hope for "every commit passes cargo clippy" as a rule, since clippy changes too often.

@Kixunil
Copy link
Collaborator

Kixunil commented Oct 24, 2022

I mean a commit performing version bump usually also signifies "the crate was released in this state". So a different ordering could be confusing (yes, we also have tags which are better, but still).

Side note: I'm also kinda intrigued by the idea of adding a commit that bumps the version to the next one with the -dev suffix right after the release.

@tcharding
Copy link
Member

Now you mention it @Kixunil, I tend to agree. Perhaps we should have the patch from #1299, followed by the three Hongfuzz patches followed by the version bump patch? I don't think its super important bit its nice to set a good precedent since we are likely going to be backporting more often soon enough.

@Kixunil
Copy link
Collaborator

Kixunil commented Oct 25, 2022

I'm thinking maybe we should fix #1343 ASAP and backport as well?

@tcharding
Copy link
Member

Perhaps so as not to put too much of a burden on @RCasatta we can do 0.29.2 with the changes in this PR then once we sort out #1343 I can do the backport and then we can release 0.29.3? (Unless of course you just love backporting so much Riccardo that you want to do it all :)

tcharding and others added 5 commits October 26, 2022 09:55
@tcharding @RCasatta
Fix clippy warnings
f857fe1 
Clippy recently upgraded and a few two new warnings types popped up in
our codebase, fix them both in a single patch so CI passes for all
commits.

1. Remove unneeded explicit borrow
2. Use `if let Some` instead of pattern match
@apoelstra @RCasatta
fuzz: disable features in honggfuzz
a6f6ead
@apoelstra @RCasatta
fuzz: remove mysteriously-not-necessary quotes from gh action script
b4f750c
@apoelstra @RCasatta
fuzz: use travis-fuzz.sh in CI
6a9b55c
@RCasatta
bump version to 0.29.2, fix test_data exclude
db8ff70 
As specified in the doc:

`foo/` matches any directory with the name `foo` anywhere in the package.
@RCasatta
Copy link
Collaborator Author

RCasatta commented Oct 26, 2022 •
edited

commits reordered, used @tcharding's clippy fixes

CI all green

@apoelstra
Copy link
Member

There are two approvals but neither is actually for the current tip of this branch.

@sanket1729 @tcharding can you re-ack?

tcharding
tcharding approved these changes Oct 26, 2022
View reviewed changes
Copy link
Member

@tcharding tcharding left a comment

Choose a reason for hiding this comment

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

Bother, I did not notice yesterday but we probably should have a changelog entry as well

# 0.29.2 - 2022-10-27

Backport Hongfuzz fixes as well as some fixes for new clippy version.

ACK db8ff70

@tcharding
Copy link
Member

Thanks @RCasatta!

apoelstra
apoelstra approved these changes Oct 26, 2022
View reviewed changes
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 db8ff70

@apoelstra apoelstra merged commit fa8ff23 into rust-bitcoin:0.29.x Oct 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@apoelstra apoelstra apoelstra approved these changes

@sanket1729 sanket1729 sanket1729 approved these changes

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

None yet
5 participants
@RCasatta @apoelstra @Kixunil @sanket1729 @tcharding