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.

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

Usage of qualifying path in types #700

Open
tcharding opened this issue Jun 21, 2024 · 3 comments
Open

Usage of qualifying path in types #700

tcharding opened this issue Jun 21, 2024 · 3 comments

Comments

@tcharding
Copy link
Member

This crate uses qualifying paths for types eg, bitcoin::PublicKey and secp256k1::PublicKey. I understand that this adds clarity, especially for types that exist in multiple places (like the given example). However qualifying paths like bitcoin::Transaction do not really add any additional clarity.

Note also that if the bitcoin_primitives crate comes into play the bitcoin:: paths will get a bunch longer/noisier (excluding using any alias magic).

Would we be open to changing the qualifying path strategy in this crate? One suggestion would be to only qualify non-bitcoin types eg., still qualify the secp256k1 types.

@apoelstra
Copy link
Member

However qualifying paths like bitcoin::Transaction do not really add any additional clarity.

I wouldn't mind dropping them here, but FWIW I think they do add clarity -- when I'm working I often also have elements::Transactions also in play. Though I appreciate this crate is bitcoin-only and everything it does is designed to work with bitcoin transactions.

Note also that if the bitcoin_primitives crate comes into play the bitcoin:: paths will get a bunch longer/noisier (excluding using any alias magic).

Yeah, this is unfortunate. I don't want to see bitcoin_primitives::Transaction everywhere. I wouldn't mind primitives::Transaction but that really does provide no value.

I guess we should just drop the bitcoin:: qualifying types. Specifically with PublicKey we have some ambiguity so maybe we should import the secp one as SecpPublicKey and then use PublicKey for the bitcoin one.

@tcharding
Copy link
Member Author

hmm, perhaps use bitcoin_primitives as bitcoin would be beneficial then, and keep bitcoin::Transaction.

@apoelstra
Copy link
Member

I think that'd be too confusing because there is a real bitcoin crate that people would think we were using.

Probably we should just drop the prefixes and use bare Transaction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants