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

Fix broken stuff #624

Merged
merged 2 commits into from
Jul 14, 2023
Merged

Fix broken stuff #624

merged 2 commits into from
Jul 14, 2023

Conversation

tcharding
Copy link
Member

@tcharding tcharding commented Jul 12, 2023

Goodness me, I made a mess.

Commit 0e0dcb7f CI: Pin dependencies required for MSRV build is totally wrong, why did it get through CI?

Fix broken stuff in the CI script by doing:

  • serde_json is not a dependency of secp256k1, remove the pinning
  • Put the pinning before any call to cargo
  • Pin the transient dependency wasm-bindgen-test

And then Revert "WIP: Add toolchain matrix to job"

Fix: #626

@tcharding
Copy link
Member Author

Rebased on #625 while trying to investigate the CI fails.

@tcharding
Copy link
Member Author

CI fails, created #626

apoelstra added a commit that referenced this pull request Jul 13, 2023
81b154f Remove docsrs cfg_attributes (Tobin C. Harding)

Pull request description:

  We no longer need to manually configure the docsrs build to highlight feature guards since we use the `doc_auto_cfg` feature. Somehow when we added usage of that feature we forgot to remove the other attributes.

  Found in CI fail of #624

ACKs for top commit:
  sanket1729:
    utACK 81b154f. I don't follow this completely, but yay for removing unnecessary code.
  apoelstra:
    ACK 81b154f

Tree-SHA512: 9decdc3f71d8f592047eee89f7f4aaf3a08b2643148c6bc5ad7de9edf61acab0ee56bf3c6dbc14493a9d089d492e31f1d379539e256b5eb96c8873c3be702256
@tcharding tcharding changed the title Revert "WIP: Add toolchain matrix to job" Fix broken stuff Jul 14, 2023
@tcharding tcharding force-pushed the 07-13-ci branch 3 times, most recently from 7f77069 to cc17276 Compare July 14, 2023 02:52
Commit `0e0dcb7f CI: Pin dependencies required for MSRV build` is
totally wrong, why did it get through CI?

In the CI script do:

- `serde_json` is not a dependency of `secp256k1`, remove the pinning
- Put the pinning _before_ any call to `cargo`
- Pin the transient dependency `wasm-bindgen-test`
This reverts commit 77808b7.

dtolnay/rust-toolchain does not support using a matrix as far as I can
tell. Since the PR brief description contains "WIP" it looks like the
original author (me) was testing this using CI, no idea how this patch
got merged.
Copy link
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 567c39c

@apoelstra apoelstra merged commit 29e1a0c into rust-bitcoin:master Jul 14, 2023
24 checks passed
@tcharding tcharding deleted the 07-13-ci branch July 17, 2023 23:39
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.

cargo feature or bug?
2 participants