Release tracking PR - v0.29.0#1109
Conversation
|
I tried to add label:"release notes mention" to aid this. But did not do so consistently. |
|
Nice idea, who do you imagine adds that label, the author wanting a mention, reviewers thinking its important enough, or the merger? |
|
I suggest removing #1114 from requirements since it doesn't affect consumer code. @tcharding I suggest the label is added by whoever thinks it should be and if there's disagreement we can discuss. I'd also like two more labels: |
|
Agreed, I don't think it's important who adds these labels -- they're unlikely to be controversial and if they are, that seems evidence enough to put something into the release notes :) |
Doing this implies that the author must add the label, I'd like to prevent burdening newer contributors with tasks that are just there to help with administrative tasks (writing the release notes). If the label is optional it at least means we give authors a chance to request that their work gets mentioned. Any PRs that don't get tagged just get left out of the release notes, no big deal. I've added the two additional labels you requested @Kixunil |
It doesn't, it implies a reviewer has to do that. However, I just realized it's possibly annoying, since a non-member would have to trigger CI re-run after the label was added.
From my experience of contributing to LND where they have very similar rule - just it has to be in a file, it's not a big deal if not for exactly these problems:
Ideally GitHub would have a native tool to resolve that but it could be done with this trick:
From my experience contributing to Rust lang, these bots are pretty nice and can do multiple things including welcoming new contributors, assigning reviewers... |
cc052ef to
6e95c9e
Compare
|
I added #1137 to required because it'd greatly suck to not have |
6e95c9e to
c065cb0
Compare
c065cb0 to
1255de6
Compare
|
Can we also add #1151 which includes a bug fix and some cleanups |
|
I've triggered re-run of CI. |
|
All, green, almost ready. However I just realized maybe #1144 would be good to include. Pros/cons:
|
|
I did go through all PRs and added missing milestone/API break markers. We should make the list of breaking changes visible. |
|
Sure, I'll try to sneak in #1144 today. |
|
It's been deprecated forever and AFAIK never had any value, I see no reason to keep it around for another cycle. |
sanket1729
left a comment
There was a problem hiding this comment.
ACK 1255de686c882408e5a11117d781cf1d783a3d39. Let's get rolling
Kixunil
left a comment
There was a problem hiding this comment.
I think we should make breaking changes more visible. Other than that ACK.
CHANGELOG.md
Outdated
There was a problem hiding this comment.
| Some highlights: | |
| ## Breaking changes | |
| There are numerous breaking changes in this release related to the new language features but also other improvements such as more newtypes added. Note that not all changes cause compilation failure! [`Witness` serialization was changed](https://github.com/rust-bitcoin/rust-bitcoin/pull/1068) to support human-readable formats. | |
| [Detailed list of breaking changes](https://github.com/rust-bitcoin/rust-bitcoin/pulls?q=is%3Apr+label%3A%22API+break%22+is%3Aclosed+milestone%3A0.29.0+) | |
| ## Highlights |
There was a problem hiding this comment.
I like the list of breaking changes, it raises a question - if we are to use this sort of link (filtered search) for each release we have to ensure all PRs have a milestone when they merge. It would be nice if this was automated so we don't miss any.
There was a problem hiding this comment.
I've added the sections you suggest, thanks. Added #1161 to highlights also.
There was a problem hiding this comment.
Yep, this time I went over them and manually modified. I'd like to avoid repeating it...
1255de6 to
87328ab
Compare
Kixunil
left a comment
There was a problem hiding this comment.
ACK 87328ab6dc0206c64747945682dbc69766c894fb
Let's goooo!
|
Should we update our rust-bitcoinconsensus dep as well? |
|
@apoelstra definitely! Good catch. I'm confused about the versioning though. I think |
|
@Kixunil rust-bitcoin/rust-bitcoinconsensus#40 everything before the As far as I'm concerned this is a problem with crates.io/cargo and there's not much we can do about it. @tcharding can you change this PR to bump the bitcoinconsensus version? |
I did it as a separate PR: #1165 This raised the question, do we want to update bitcoinconsensus to use bitcoin core 21 (taproot release) and depend on that before this release or does that not matter? For the record I have an open PR to update bitcoinconsensus but its broken, working on fixing it now. |
|
Maybe we should just go ahead and release with bitcoinconsensus 0.20.2-0.5.0 because I cannot get the upgrade to bitcoin core 21 working. |
|
Agreed. I'd like to have 21 but I don't think it's a blocker, and if it's nontrivial to do then we can let it slip. |
|
I added P-high label to #1165, so we can get that through. I'll push update to release notes and rebase soon as that one merges. Then we should be about right to go, anything else we are missing? |
|
@tcharding merged #1165 . I think we are good to go |
Add changelog notes and bump the version number to v0.29.0.
87328ab to
110b5d8
Compare
|
Rebased and added chagelog entry for bitcoinconsensus dependency upgrade - lets go! |
|
Oh, can we get #1171 in? It's trivial and would be nice to combine it with having |
|
OK, the two remaining checklist questions are #1106 :
|
|
Will wait for one more ACK then let's go! |
|
I suspect that semver trick is too complex for these changes but I'm willing to review if someone figures it out. Also upgrading guide doesn't feel required since we were pretty careful about things like people accidentally unwrapping stuff. (Maybe that part can only be decided by the number of "I can't upgrade" issues.) |
|
Agreed that we won't be able to e.g. semver-trick But we could semver-trick e.g. the entire |
|
Hmm, partial semver trick may be useful too. I'm not sure how much it helps though. |
|
@Kixunil i think we could partial-semver-trick |
|
Oh, |
|
BTW @tcharding could we have an ACK on this? I don't wanna publish without your awareness. |
|
ACK 110b5d8 Thanks! |
|
Let's go! |
|
Published, signed and tagged. |
Add changelog notes and bump the version number to v0.29.0.
Notes
I attempted to go through all the PRs since last release, please sing out if you had a PR merged that is not mentioned and you would like it mentioned.
The changelog notes can be changed or improved, please do not take me writing them to imply I know exactly what goes on round here - I just made an effort to save others having to do it.
TODO
CommandString#1137