-
Notifications
You must be signed in to change notification settings - Fork 139
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.
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
Release tracking PR: v12.0.0
#688
Conversation
532f314
to
f2a61ad
Compare
@apoelstra and/or @sanket1729 can you guys please look closer than usual at the changelog entries because I don't know what goes on here as closely as I do in other crates, specifically are the sub-headings correct and/or useful? I basically included every merged PR since last release. Thanks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you've asked for eyes on the changelogs, I've added comments.
Most of these is for consistency throughout the new changelog.
CHANGELOG.md
Outdated
|
||
- Update MSRV to Rust 1.56.1 [#639](https://github.com/rust-bitcoin/rust-miniscript/pull/639) | ||
- Drop the `Property` trait entirely [#652](https://github.com/rust-bitcoin/rust-miniscript/pull/652) | ||
- Improve compiler logic when deciding between conjunctions and multi/multi_a [#657](https://github.com/rust-bitcoin/rust-miniscript/pull/657) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Improve compiler logic when deciding between conjunctions and multi/multi_a [#657](https://github.com/rust-bitcoin/rust-miniscript/pull/657) | |
- Improve compiler logic when deciding between conjunctions and `multi`/`multi_a` [#657](https://github.com/rust-bitcoin/rust-miniscript/pull/657) |
CHANGELOG.md
Outdated
- Drop the `Property` trait entirely [#652](https://github.com/rust-bitcoin/rust-miniscript/pull/652) | ||
- Improve compiler logic when deciding between conjunctions and multi/multi_a [#657](https://github.com/rust-bitcoin/rust-miniscript/pull/657) | ||
- Several locktime improvements [#654](https://github.com/rust-bitcoin/rust-miniscript/pull/654) | ||
- Derives `Hash` for pub items [#659](https://github.com/rust-bitcoin/rust-miniscript/pull/659) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Derives `Hash` for pub items [#659](https://github.com/rust-bitcoin/rust-miniscript/pull/659) | |
- Derives `Hash` for `pub` items [#659](https://github.com/rust-bitcoin/rust-miniscript/pull/659) |
CHANGELOG.md
Outdated
- Drop the `Property` trait entirely [#652](https://github.com/rust-bitcoin/rust-miniscript/pull/652) | ||
- Improve compiler logic when deciding between conjunctions and multi/multi_a [#657](https://github.com/rust-bitcoin/rust-miniscript/pull/657) | ||
- Several locktime improvements [#654](https://github.com/rust-bitcoin/rust-miniscript/pull/654) | ||
- Derives `Hash` for pub items [#659](https://github.com/rust-bitcoin/rust-miniscript/pull/659) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Derives `Hash` for pub items [#659](https://github.com/rust-bitcoin/rust-miniscript/pull/659) | |
- Derive `Hash` for pub items [#659](https://github.com/rust-bitcoin/rust-miniscript/pull/659) |
Why all other are like "Introduce", "Upgrade", "Return" etc; and this is "Derives"
- Introduce `Threshold` type [#660](https://github.com/rust-bitcoin/rust-miniscript/pull/660), | ||
[#674](https://github.com/rust-bitcoin/rust-miniscript/pull/674), | ||
and [#676](https://github.com/rust-bitcoin/rust-miniscript/pull/676) | ||
- Upgrade `bech32` dependency to `v0.11.0` [#661](https://github.com/rust-bitcoin/rust-miniscript/pull/661) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Either this version is non code, i.e. 0.11.0 or the MSRV Rust version above should be as code (backticks) for consistency
CHANGELOG.md
Outdated
## Performance/compiled time improvements | ||
|
||
- Remove recursion in `semantic` module [#612](https://github.com/rust-bitcoin/rust-miniscript/pull/612) | ||
- Remove generics from Error by making fragment a String [#642](https://github.com/rust-bitcoin/rust-miniscript/pull/642) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Remove generics from Error by making fragment a String [#642](https://github.com/rust-bitcoin/rust-miniscript/pull/642) | |
- Remove generics from `Error` by making fragment a `String` [#642](https://github.com/rust-bitcoin/rust-miniscript/pull/642) |
CHANGELOG.md
Outdated
|
||
- Remove recursion in `semantic` module [#612](https://github.com/rust-bitcoin/rust-miniscript/pull/612) | ||
- Remove generics from Error by making fragment a String [#642](https://github.com/rust-bitcoin/rust-miniscript/pull/642) | ||
- Remove unused generic on check_witness [#644](https://github.com/rust-bitcoin/rust-miniscript/pull/644) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Remove unused generic on check_witness [#644](https://github.com/rust-bitcoin/rust-miniscript/pull/644) | |
- Remove unused generic on `check_witness` [#644](https://github.com/rust-bitcoin/rust-miniscript/pull/644) |
CHANGELOG.md
Outdated
- Remove recursion in `semantic` module [#612](https://github.com/rust-bitcoin/rust-miniscript/pull/612) | ||
- Remove generics from Error by making fragment a String [#642](https://github.com/rust-bitcoin/rust-miniscript/pull/642) | ||
- Remove unused generic on check_witness [#644](https://github.com/rust-bitcoin/rust-miniscript/pull/644) | ||
- Add conditional formatting for Terminal [#651](https://github.com/rust-bitcoin/rust-miniscript/pull/651) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Add conditional formatting for Terminal [#651](https://github.com/rust-bitcoin/rust-miniscript/pull/651) | |
- Add conditional formatting for `Terminal` [#651](https://github.com/rust-bitcoin/rust-miniscript/pull/651) |
CHANGELOG.md
Outdated
|
||
- Remove `internals` dependency [](https://github.com/rust-bitcoin/rust-miniscript/pull/631) | ||
- Introduce an example binary useful for profiling [#646](https://github.com/rust-bitcoin/rust-miniscript/pull/646) | ||
- Refactor out type_check [#649](https://github.com/rust-bitcoin/rust-miniscript/pull/649) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Refactor out type_check [#649](https://github.com/rust-bitcoin/rust-miniscript/pull/649) | |
- Refactor out `type_check` [#649](https://github.com/rust-bitcoin/rust-miniscript/pull/649) |
Thanks @storopoli, I'll re-spin with all your suggestions. |
f2a61ad
to
15e3d8a
Compare
Hey @storopoli man, to save you review time (which is precious) I would have accepted "you forgot to back tick all the types". But thanks for taking the time to point each out. I also discovered a set of empty |
15e3d8a
to
7ddaaf6
Compare
All yours @apoelstra - review, ack, merge, tag, and publish please (if the changelog is acceptable). |
CHANGELOG.md
Outdated
- Improve compiler logic when deciding between conjunctions and `multi`/`multi_a` [#657](https://github.com/rust-bitcoin/rust-miniscript/pull/657) | ||
- Several locktime improvements [#654](https://github.com/rust-bitcoin/rust-miniscript/pull/654) | ||
- Derive `Hash` for `pub` items [#659](https://github.com/rust-bitcoin/rust-miniscript/pull/659) | ||
- Introduce `Threshold` type [#660](https://github.com/rust-bitcoin/rust-miniscript/pull/660), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In 7ddaaf6:
Trailing whitespace, and also a trailing comma.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
\me Puts coin in money jar for not reading diff before pushing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That jar is getting full... Reminds me a calculus teacher that had a jar of coins for everytime that a student forgot to add
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a swear jar once but it got full so quick I stopped. Now we do 5 pushups each time instead - works way better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ha! Just did five push ups and said "read your diff" at the top of each one - lets see how it goes.
7ddaaf6
to
df113f6
Compare
Note, I did not rebase on |
Also, added #645 but did not add additional explanation - ok? |
|
In preparation for release add a changelog entry and bump the version.
df113f6
to
5ea33e3
Compare
Updated date to 22nd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 5ea33e3
Tagged and published. |
In preparation for release add a changelog entry and bump the version.