Skip to content

units: Test for dyn compatibility#3926

Merged
apoelstra merged 1 commit intorust-bitcoin:masterfrom
tcharding:01-20-units-dyn-compatible
Jan 20, 2025
Merged

units: Test for dyn compatibility#3926
apoelstra merged 1 commit intorust-bitcoin:masterfrom
tcharding:01-20-units-dyn-compatible

Conversation

@tcharding
Copy link
Copy Markdown
Member

Phew! dyn compatibility is a non-trivial concept. There are four public traits in units, only one is dyn compatible.

This patch is done in order to check off C-OBJECT from the Rust API guidelines checklist.

Add a test to check the public traits in units for dyn compatibility.

While we are at it add a code comment on Integer stating why its not dyn-compatible.

ref: https://rust-lang.github.io/api-guidelines/flexibility.html#c-object

Phew! dyn compatibility is a non-trivial concept. There are four public
traits in `units`, only one is dyn compatible.

This patch is done in order to check off C-OBJECT from the Rust API
guidelines checklist.

Add a test to check the public traits in `units` for dyn compatibility.

While we are at it add a code comment on `Integer` stating why its not
dyn-compatible.

ref: https://rust-lang.github.io/api-guidelines/flexibility.html#c-object
@github-actions github-actions bot added C-units PRs modifying the units crate test labels Jan 19, 2025
@tcharding
Copy link
Copy Markdown
Member Author

Please note, I already checked off C-OBJECT in #3632 - this PR adds a test that proves it. When this merges we can check off units in #3829.

tcharding added a commit to tcharding/rust-bitcoin that referenced this pull request Jan 20, 2025
In preparation for release add a changelog entry, bump the version, and
update the lock files.

`v1.0` here we come.

Before we merge this we should add:

- rust-bitcoin#3934 or rust-bitcoin#3866
- rust-bitcoin#3933
- rust-bitcoin#3932
- rust-bitcoin#3929
- rust-bitcoin#3926
- rust-bitcoin#3923
- rust-bitcoin#3893
- rust-bitcoin#3866
- rust-bitcoin#3794
Copy link
Copy Markdown
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 289a521; successfully ran local tests; nice!

@apoelstra apoelstra merged commit 8ff69fe into rust-bitcoin:master Jan 20, 2025
@tcharding tcharding deleted the 01-20-units-dyn-compatible branch January 21, 2025 23:03
tcharding added a commit to tcharding/rust-bitcoin that referenced this pull request Feb 24, 2025
We are trying a new strategy to get to 1.0 more quickly - remove
`amount` and `fee` and release everything else.

In preparation for release add a changelog entry, bump the version, and
update the lock files.

`v1.0` here we come.

Before we merge this we should add:

- rust-bitcoin#3934 or rust-bitcoin#3866
- rust-bitcoin#3933
- rust-bitcoin#3932
- rust-bitcoin#3929
- rust-bitcoin#3926
- rust-bitcoin#3923
- rust-bitcoin#3893
- rust-bitcoin#3866
- rust-bitcoin#3794
tcharding added a commit to tcharding/rust-bitcoin that referenced this pull request Feb 24, 2025
We are trying a new strategy to get to 1.0 more quickly - remove
`amount` and `fee` and release everything else.

In preparation for release add a changelog entry, bump the version, and
update the lock files.

`v1.0` here we come.

Before we merge this we should add:

- rust-bitcoin#3934 or rust-bitcoin#3866
- rust-bitcoin#3933
- rust-bitcoin#3932
- rust-bitcoin#3929
- rust-bitcoin#3926
- rust-bitcoin#3923
- rust-bitcoin#3893
- rust-bitcoin#3866
- rust-bitcoin#3794
tcharding added a commit to tcharding/rust-bitcoin that referenced this pull request Feb 24, 2025
We are trying a new strategy to get to 1.0 more quickly - remove
`amount` and `fee` and release everything else.

In preparation for release add a changelog entry, bump the version, and
update the lock files.

`v1.0` here we come.

Before we merge this we should add:

- rust-bitcoin#3934 or rust-bitcoin#3866
- rust-bitcoin#3933
- rust-bitcoin#3932
- rust-bitcoin#3929
- rust-bitcoin#3926
- rust-bitcoin#3923
- rust-bitcoin#3893
- rust-bitcoin#3866
- rust-bitcoin#3794
tcharding added a commit to tcharding/rust-bitcoin that referenced this pull request Feb 24, 2025
We are trying a new strategy to get to 1.0 more quickly - remove
`amount` and `fee` and release everything else.

In preparation for release add a changelog entry, bump the version, and
update the lock files.

`v1.0` here we come.

Before we merge this we should add:

- rust-bitcoin#3934 or rust-bitcoin#3866
- rust-bitcoin#3933
- rust-bitcoin#3932
- rust-bitcoin#3929
- rust-bitcoin#3926
- rust-bitcoin#3923
- rust-bitcoin#3893
- rust-bitcoin#3866
- rust-bitcoin#3794
tcharding added a commit to tcharding/rust-bitcoin that referenced this pull request Feb 24, 2025
We are trying a new strategy to get to 1.0 more quickly - remove
`amount` and `fee` and release everything else.

In preparation for release add a changelog entry, bump the version, and
update the lock files.

`v1.0` here we come.

Before we merge this we should add:

- rust-bitcoin#3934 or rust-bitcoin#3866
- rust-bitcoin#3933
- rust-bitcoin#3932
- rust-bitcoin#3929
- rust-bitcoin#3926
- rust-bitcoin#3923
- rust-bitcoin#3893
- rust-bitcoin#3866
- rust-bitcoin#3794
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-units PRs modifying the units crate test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants