Move impl_array_newtype to internals
#5334
Conversation
github-actions
bot
added
C-bitcoin
Perhaps remove it from the macro but put an impl block on each type that calls the macro with the deprecated function. I was hesitant when I read the PR title because of my distrust of macros, but this one is pretty good. So FWIW concept ACK that. Will review. |
rustaceanrob
force-pushed
the
11-26-mv-nt
branch
from
964b66f to
697789d
Compare
|
Moved deprecated method to I neglected the method in |
rockcoolsaint
left a comment
rockcoolsaint
left a comment
There was a problem hiding this comment.
The changes moved without breaking existing tests
|
Agreed, this macro seems okay. Importantly it does not use anything from other crates except core/std. concept ACK. |
|
ACK 697789d |
|
@tcharding when you get a moment, you said "will review" so I'll hold off on merging til you do. |
|
Oooph, sorry I meant to do it that day. |
bitcoin/src/bip32.rs
Outdated
| internal_macros::impl_array_newtype!(ChainCode, u8, 32); | ||
| impl_array_newtype!(ChainCode, u8, 32); |
There was a problem hiding this comment.
nit: I'd prefer one level of path, wi have been working towards doing so for macros repo wide.
internals::impl_arry_newtyp! ...
rustaceanrob
dismissed stale reviews from tcharding and apoelstra
via
8b4c773
rustaceanrob
force-pushed
the
11-26-mv-nt
branch
from
697789d to
8b4c773
Compare
Discussion rust-bitcoin#5331 suggested removing `bitcoin` as a direct dependency could be a reasonable goal for the `p2p` crate. One conflict along the way is the use of this macro in `bip152` to implement the array-like traits for the short IDs. Because `bitcoin` depends on `internals`, as does `p2p`, we can move this macro into `internals` to help detangle `p2p` and `bitcoin`. Moves the deprecated methods to the `impl` blocks.
rustaceanrob
force-pushed
the
11-26-mv-nt
branch
from
8b4c773 to
20cd232
Compare
|
Addressed all the imports in 20cd232 |
|
Thanks mate. |
5 participants
Discussion #5331 suggested removing
bitcoinas a direct dependency could be a reasonable goal for thep2pcrate. One conflict along the way is the use of this macro inbip152to implement the array-like traits for the short IDs. Becausebitcoindepends oninternals, as doesp2p, we can move this macro intointernalsto help detanglep2pandbitcoin.Does it make sense to also remove the deprecated method?