Fix unused arg in PSBT impl_psbt_get_pair macro#786
Merged
apoelstra merged 1 commit intorust-bitcoin:masterfrom Jan 17, 2022
BP-WG:psbt/macro_fix
Merged
Fix unused arg in PSBT impl_psbt_get_pair macro#786apoelstra merged 1 commit intorust-bitcoin:masterfrom BP-WG:psbt/macro_fix
apoelstra merged 1 commit intorust-bitcoin:masterfrom
BP-WG:psbt/macro_fix
Conversation
apoelstra
previously approved these changes
Jan 15, 2022
Member
apoelstra
left a comment
There was a problem hiding this comment.
ACK e8a06fd41d29607f7eca5074581d010fe5428d37
sanket1729
reviewed
Jan 16, 2022
src/util/psbt/macros.rs
Outdated
Member
There was a problem hiding this comment.
nit: Remove the Option everywhere?
src/util/psbt/macros.rs
Outdated
Member
There was a problem hiding this comment.
We can also remove the BTreeMap here, and change this to rv.push and the above can be rv.push_map. The above types Option and BTreeMap are merely matching the syntax and not used in the macro.
If you do the above suggestions, the code might look simpler:
rv.push(self.witness_script as PSBT_OUT_WITNESS_SCRIPT)
rv.push_map(self.bip32_derivation as PSBT_OUT_BIP32_DERIVATION)
Merged
Collaborator
Author
|
@sanket1729 simplified as you suggested - also removed unnecessary/confusing |
apoelstra
approved these changes
Jan 17, 2022
Member
apoelstra
left a comment
There was a problem hiding this comment.
ACK 1b77e36
Good idea @sanket1729 !
Member
|
Let's get one more ACK just in case others have opinions about these macros. |
apoelstra
added a commit
that referenced
this pull request
Jan 18, 2022
1b2dbd7 0.28.0-rc.1 release (Dr Maxim Orlovsky) Pull request description: We still have quite a few issues and PRs pending to be addressed/merged before full 0.28 release: see https://github.com/rust-bitcoin/rust-bitcoin/milestone/10 Most important, we have to find a way to address #777; additionally it will be desirable to get #587, #786, #776. We also have quite of work to write CHANGELOG in #785 Nevertheless I propose to start with a `beta-1` subrelease to unlock ability for `miniscript` release and downstream testing. The Taproot API looks final, and the pending PRs are addresses internal issues/bugs which is perfectly fine for beta releases. ACKs for top commit: Kixunil: ACK 1b2dbd7 Tree-SHA512: a7bd6dd3e7a489f7fd758fb0d7a60103bfe0cdf88997b2163f163841fa1c12e7b31fa8f03b9f817a671379578dbc10a2ca583f49b64d2d2de4a9ebb57b2b9bd5
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #754