Skip to content

hashes: Add an api test module#4017

Merged
apoelstra merged 1 commit intorust-bitcoin:masterfrom
tcharding:05-06-hashes-api-test
Apr 16, 2025
Merged

hashes: Add an api test module#4017
apoelstra merged 1 commit intorust-bitcoin:masterfrom
tcharding:05-06-hashes-api-test

Conversation

@tcharding
Copy link
Copy Markdown
Member

@tcharding tcharding commented Feb 6, 2025

As we did for units and as part of the stabalization effort.

Add an api test module that verifies the public API for the hashes crate.

Close: #3927

@github-actions github-actions bot added C-bitcoin PRs modifying the bitcoin crate C-hashes PRs modifying the hashes crate test labels Feb 6, 2025
@github-actions
Copy link
Copy Markdown

github-actions bot commented Feb 6, 2025

🚨 API BREAKING CHANGE DETECTED

To see the changes click details on "Check semver breaks / PR Semver - stable toolchain" job then expand "Run semver checker script" and scroll to the end of the section.

@github-actions github-actions bot added the API break This PR requires a version bump for the next release label Feb 6, 2025
@tcharding tcharding force-pushed the 05-06-hashes-api-test branch from ffe6afe to b3bb8d0 Compare February 11, 2025 03:31
@github-actions github-actions bot added C-primitives and removed C-bitcoin PRs modifying the bitcoin crate labels Feb 11, 2025
@tcharding tcharding marked this pull request as ready for review February 11, 2025 03:31
@github-actions
Copy link
Copy Markdown

🚨 API BREAKING CHANGE DETECTED

To see the changes click details on "Check semver breaks / PR Semver - stable toolchain" job then expand "Run semver checker script" and scroll to the end of the section.

@github-actions
Copy link
Copy Markdown

🚨 API BREAKING CHANGE DETECTED

To see the changes click details on "Check semver breaks / PR Semver - stable toolchain" job then expand "Run semver checker script" and scroll to the end of the section.

@tcharding tcharding marked this pull request as draft February 11, 2025 08:38
@tcharding
Copy link
Copy Markdown
Member Author

Win for the API breaking bot - I botched this, it should not have API changes mixed into a single patch. Will fix tomorrrow.

@tcharding tcharding force-pushed the 05-06-hashes-api-test branch from 1ae7a57 to b570f8b Compare February 11, 2025 16:32
@tcharding tcharding marked this pull request as ready for review February 11, 2025 16:32
@tcharding tcharding force-pushed the 05-06-hashes-api-test branch from b570f8b to 9fd09bf Compare February 13, 2025 02:46
@github-actions
Copy link
Copy Markdown

🚨 API BREAKING CHANGE DETECTED

To see the changes click details on "Check semver breaks / PR Semver - stable toolchain" job then expand "Run semver checker script" and scroll to the end of the section.

@tcharding tcharding marked this pull request as draft February 13, 2025 02:48
@tcharding
Copy link
Copy Markdown
Member Author

tcharding commented Feb 14, 2025

Note to self, the PRs on which this is based have changed. Be careful when rebasing.

Hey @apoelstra, @Kixunil, @jamillambert - are these comments I write to myself annoying when they show up in your notifications?

@Kixunil
Copy link
Copy Markdown
Collaborator

Kixunil commented Feb 14, 2025

I usually ignore drafts. It's a random chance that I even looked at this one.

@jamillambert
Copy link
Copy Markdown
Contributor

I don't mind. If you hadn't mentioned me I would only see them when I was in the mood to look through the PR and probably find them helpful to see the status of it.

@apoelstra
Copy link
Copy Markdown
Member

TL;DR I don't mind.

Once my local CI box has a few hours of work queued up then I start looking at issues and drafts, since those can be moved out of my notifications and open tabs without waiting for the CI box.

Usually with drafts I don't even look at the code, on the assumption that I'll get a notification later when it gets undrafted or when somebody specifically asks me to look at the code.

@tcharding
Copy link
Copy Markdown
Member Author

Mad, cheers.

@tcharding tcharding force-pushed the 05-06-hashes-api-test branch from 9fd09bf to 2421cf4 Compare February 17, 2025 02:07
tcharding added a commit to tcharding/rust-bitcoin that referenced this pull request Feb 19, 2025
In preparation for 1.0-ing `primitives` add an `api` test module that
makes an effort to verify the API surface.

This is similar to what is in `units` and what is in development for
`hashes` (in rust-bitcoin#4017).
tcharding added a commit to tcharding/rust-bitcoin that referenced this pull request Feb 19, 2025
In preparation for 1.0-ing `primitives` add an `api` test module that
makes an effort to verify the API surface.

This is similar to what is in `units` and what is in development for
`hashes` (in rust-bitcoin#4017).
@tcharding
Copy link
Copy Markdown
Member Author

O boy, if we are considering doing #4051 then this trait bound discussion is a waste of clock cycles. Removing the patch and requesting merge of this as is. I'll amend the FIXME to a plain comment.

@tcharding tcharding force-pushed the 05-06-hashes-api-test branch from 4ee52bd to 9c23ec6 Compare February 19, 2025 23:39
@tcharding tcharding marked this pull request as ready for review February 19, 2025 23:39
@github-actions github-actions bot removed the C-io PRs modifying the io crate label Feb 19, 2025
@tcharding tcharding marked this pull request as draft February 20, 2025 03:52
@tcharding
Copy link
Copy Markdown
Member Author

Lets let #4085 go in first.

tcharding added a commit to tcharding/rust-bitcoin that referenced this pull request Feb 20, 2025
In preparation for 1.0-ing `primitives` add an `api` test module that
makes an effort to verify the API surface.

This is similar to what is in `units` and what is in development for
`hashes` (in rust-bitcoin#4017).
@tcharding tcharding force-pushed the 05-06-hashes-api-test branch from 9c23ec6 to 926b061 Compare March 6, 2025 00:57
@github-actions github-actions bot added C-bitcoin PRs modifying the bitcoin crate C-io PRs modifying the io crate labels Mar 6, 2025
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 6, 2025

🚨 API BREAKING CHANGE DETECTED

To see the changes click details on "Check semver breaks / PR Semver - stable toolchain" job then expand "Run semver checker script" and scroll to the end of the section.

tcharding added a commit to tcharding/rust-bitcoin that referenced this pull request Mar 20, 2025
In preparation for 1.0-ing `primitives` add an `api` test module that
makes an effort to verify the API surface.

This is similar to what is in `units` and what is in development for
`hashes` (in rust-bitcoin#4017).
tcharding added a commit to tcharding/rust-bitcoin that referenced this pull request Mar 20, 2025
In preparation for 1.0-ing `primitives` add an `api` test module that
makes an effort to verify the API surface.

This is similar to what is in `units` and what is in development for
`hashes` (in rust-bitcoin#4017).
tcharding added a commit to tcharding/rust-bitcoin that referenced this pull request Mar 20, 2025
In preparation for 1.0-ing `primitives` add an `api` test module that
makes an effort to verify the API surface.

This is similar to what is in `units` and what is in development for
`hashes` (in rust-bitcoin#4017).
As we did for `units` and as part of the stabalization effort.

Add an `api` test module that verifies the public API for the `hashes`
crate.
@tcharding tcharding force-pushed the 05-06-hashes-api-test branch from 926b061 to f268ca2 Compare April 10, 2025 20:51
@tcharding tcharding marked this pull request as ready for review April 10, 2025 20:51
@github-actions github-actions bot removed C-bitcoin PRs modifying the bitcoin crate C-io PRs modifying the io crate labels Apr 10, 2025
tcharding added a commit to tcharding/rust-bitcoin that referenced this pull request Apr 10, 2025
In preparation for 1.0-ing `primitives` add an `api` test module that
makes an effort to verify the API surface.

This is similar to what is in `units` and what is in development for
`hashes` (in rust-bitcoin#4017).
tcharding added a commit to tcharding/rust-bitcoin that referenced this pull request Apr 11, 2025
In preparation for 1.0-ing `primitives` add an `api` test module that
makes an effort to verify the API surface.

This is similar to what is in `units` and what is in development for
`hashes` (in rust-bitcoin#4017).
@apoelstra
Copy link
Copy Markdown
Member

Looks good! It does make me think that maybe we can implement Hash for Hkdf. Though I'm unsure how useful that is without PartialEq, and I'm not sure if we want PartialEq if we need to implement it in a slow/constant-time way.

Maybe better to wait for somebody to show up with a usecase and we can add stuff then..

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 f268ca2; successfully ran local tests

@tcharding
Copy link
Copy Markdown
Member Author

Shameless bump.

@apoelstra apoelstra merged commit 0afe65f into rust-bitcoin:master Apr 16, 2025
24 checks passed
tcharding added a commit to tcharding/rust-bitcoin that referenced this pull request May 8, 2025
In preparation for 1.0-ing `primitives` add an `api` test module that
makes an effort to verify the API surface.

This is similar to what is in `units` and what is in development for
`hashes` (in rust-bitcoin#4017).
tcharding added a commit to tcharding/rust-bitcoin that referenced this pull request May 8, 2025
In preparation for 1.0-ing `primitives` add an `api` test module that
makes an effort to verify the API surface.

This is similar to what is in `units` and what is in development for
`hashes` (in rust-bitcoin#4017).
apoelstra added a commit that referenced this pull request May 9, 2025
e2d9a8a primitives: Add an API test module (Tobin C. Harding)
8ec2d35 primitives: Derive Clone on witness::Iter (Tobin C. Harding)

Pull request description:

  In preparation for 1.0-ing `primitives` add an `api` test module that makes an effort to verify the API surface.

  This is similar to what is in `units` and what is in development for `hashes` (in #4017).

  Note, there is a WIP attempt at this in #3992.

  Close: #3928

ACKs for top commit:
  apoelstra:
    ACK e2d9a8a; successfully ran local tests

Tree-SHA512: 5ec5c87c9aa5e86e579283a5485dcb2b3b5ae59359ae5ab96f8e6634285072bef0d0f111b6780852fd88fe29677f1a84c791a3343a0cb2b09093e77125f3962b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

API break This PR requires a version bump for the next release C-hashes PRs modifying the hashes crate test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

hashes: Add an API test module

4 participants