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

Unroll some implementations generic over ledger::Bitcoin #1992

Merged
merged 1 commit into from Feb 6, 2020

Conversation

@thomaseizinger
Copy link
Member

thomaseizinger commented Feb 5, 2020

If we want to split a comit crate out of cnd, we cannot abstract over a trait that is not defined in our crate.

This patch removes all the usages of the ledger::Bitcoin that are used in code that would be outside of the comit crate for abstracting over any bitcoin network.

Split out of #1970.

If we want to split a comit crate out of cnd, we cannot abstract
over a trait that is not defined in our crate.

This patch removes all the usages of the ledger::Bitcoin that are
used in code that would be outside of the comit crate for abstracting
over any bitcoin network.
@mergify

This comment has been minimized.

Copy link
Contributor

mergify bot commented Feb 5, 2020

Are you sure the changelog does not need updating?

@D4nte
D4nte approved these changes Feb 6, 2020
@mergify

This comment has been minimized.

Copy link
Contributor

mergify bot commented Feb 6, 2020

bors r+

bors bot added a commit that referenced this pull request Feb 6, 2020
Merge #1992
1992: Unroll some implementations generic over ledger::Bitcoin r=mergify[bot] a=thomaseizinger

If we want to split a comit crate out of cnd, we cannot abstract over a trait that is not defined in our crate.

This patch removes all the usages of the ledger::Bitcoin that are used in code that would be outside of the comit crate for abstracting over any bitcoin network.

Split out of #1970.

Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
@bors

This comment has been minimized.

Copy link
Contributor

bors bot commented Feb 6, 2020

@bors bors bot merged commit 9acd998 into master Feb 6, 2020
5 checks passed
5 checks passed
automation
Details
Summary 3 rules match
Details
bors Build succeeded
Details
ci/circleci: debug-build-test Your tests passed on CircleCI!
Details
license/cla Contributor License Agreement is signed.
Details
@mergify mergify bot deleted the unroll-bitcoin-trait branch Feb 6, 2020
@D4nte

This comment has been minimized.

Copy link
Member

D4nte commented Feb 6, 2020

If we want to split a comit crate out of cnd, we cannot abstract over a trait that is not defined in our crate.

This patch removes all the usages of the ledger::Bitcoin that are used in code that would be outside of the comit crate for abstracting over any bitcoin network.

Split out of #1970.

Why are you doing this change again?

@thomaseizinger

This comment has been minimized.

Copy link
Member Author

thomaseizinger commented Feb 9, 2020

If we want to split a comit crate out of cnd, we cannot abstract over a trait that is not defined in our crate.
This patch removes all the usages of the ledger::Bitcoin that are used in code that would be outside of the comit crate for abstracting over any bitcoin network.
Split out of #1970.

Why are you doing this change again?

You mean splitting out a comit crate?
To enforce architectural boundaries :)
But it is not high priority, just something I wanted to do for a long time and I think it is going to be beneficial in understanding and extending the codebase.

@D4nte

This comment has been minimized.

Copy link
Member

D4nte commented Feb 10, 2020

If we want to split a comit crate out of cnd, we cannot abstract over a trait that is not defined in our crate.
This patch removes all the usages of the ledger::Bitcoin that are used in code that would be outside of the comit crate for abstracting over any bitcoin network.
Split out of #1970.

Why are you doing this change again?

You mean splitting out a comit crate?
To enforce architectural boundaries :)
But it is not high priority, just something I wanted to do for a long time and I think it is going to be beneficial in understanding and extending the codebase.

No I meant, why is this PR needed to split out a comit crate?

@thomaseizinger

This comment has been minimized.

Copy link
Member Author

thomaseizinger commented Feb 10, 2020

Because you can't do a blanket impl over a trait that is defined in a different crate.
All of the above modified code would be outside the comit crate.

@D4nte

This comment has been minimized.

Copy link
Member

D4nte commented Feb 11, 2020

Because you can't do a blanket impl over a trait that is defined in a different crate.

Oh, I did not know/forgot about that. I just knew you could not implement a foreign trait on a foreign type.

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

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.