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

Accidentally closed: ~Release v0.28.0~ #621

Closed
wants to merge 14 commits into from

Conversation

tcharding
Copy link
Member

@tcharding tcharding commented Jun 27, 2023

We need a branch with this change so we can depend on it to build bitcoin while updating the hashes dependency there.

The last two patches are WIP because:

  • Patch 13: includes a patch section in the manifest to depend on rust-bitcoin/rust-bitcoin@37d1d28
  • Patch 14: the lock files will need updating one hashes is released

Pinning is broken again, update the pins it CI so that the following
sequence of commands would work

```bash
rm Cargo.lock
cargo +1.48 update -p wasm-bindgen-test --precise 0.3.34
cargo +1.48 update -p serde_test --precise 1.0.175
cargo +1.48 test --all-features
```

Note, solely out of interest, `cargo +1.48 build` does not need
pinning (at the moment :)
`cargo +nightly` output of panic recently changed breaking our grep
statement by adding the code line and a newline.

Grep for the exact second line of the error message.
Remove the `internal_features` attribute, not sure what it was supposed
to be doing but the crate works without it.
We have an unconditional panic for some combination of features, this
causes clippy to give a bunch of useless warnings.

Add allow attributes to quieten down clippy.
Currently the panic message refers to stuff related to development of
the library, this is meaningless for users of the lib. Target panic
message at secp users instead.
A while back we added two lock files that were to track dependencies of
two successful test runs, one with a minimal set of dependencies and one
with a recent set of dependencies (ie, recent dependency versions). We
never used these lock files in CI however.

In preparation for using the lock files in CI, and in order to be
uniform with `rust-bitcoin`, move the lock files to the crate root and
rename them to:

- Cargo-minimal.lock
- Cargo-recent.lock
We would like to be able to run the test script with different lock
files, in preparation for doing so move the `test.sh` script to
`_test.sh` and add a new `test.sh` that runs `_test.sh`.

Keep the outer script as `test.sh` so that we do not change the workflow
for those running the script including the github actions.
The `wasm-pack` command does not honour `cargo` flags passed to it so we
cannot use `--locked` and test against pre-made lock files. Instead just
run the WASM test from the test script wrapper.
Update the CI scripts to use the minimal/recent lockfiles, requires
using `--locked` for various `cargo` incantations.
Use the more terse `hashes` by way of the `package` field in the
manifest.

Allows us to remove the ugly feature alias "bitcoin-hashes" ->
"bitcoin_hashes" and removes all the bother with the underscore.

Why did we not think of this 2 years ago?
Now that we have `hashes` as the crate name of `bitcoin_hashes` we can
slightly clean up the import statements.

This is based on the convention we have to import things directly from
the crate if we depend on it and not from the crate level re-export.
WIP because the lock file changes will need redoing once hashes is
released.

Prepare for release of 0.28.0 by doing:

- Bump the version number in the manifest
- Add changelog entry
- Update the commited minimal/recent lock files
@tcharding tcharding changed the title WIP: Update the hashes dependency to v0.13 Release v0.28.0 Aug 9, 2023
@tcharding tcharding closed this Aug 24, 2023
@tcharding tcharding deleted the 06-28-secp-release branch August 24, 2023 02:35
@tcharding
Copy link
Member Author

WTF I did not close that, unless I fat fingered it.

@tcharding tcharding changed the title Release v0.28.0 Accidentally closed: ~Release v0.28.0~ Aug 24, 2023
@tcharding
Copy link
Member Author

Replaced by #650

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

Successfully merging this pull request may close these issues.

1 participant