Skip to content

Remove schemars all together #3395

Merged
apoelstra merged 1 commit intorust-bitcoin:masterfrom
tcharding:09-21-rm-schemars-test
Sep 24, 2024
Merged

Remove schemars all together #3395
apoelstra merged 1 commit intorust-bitcoin:masterfrom
tcharding:09-21-rm-schemars-test

Conversation

@tcharding
Copy link
Copy Markdown
Member

@tcharding tcharding commented Sep 20, 2024

We introduced schemars as a personal favor to a user, and it broke our CI repeatedly but eventually it seemed like it was stable (mainly, our MSRV caught up with its MSRV) so we just let it slide. In the end having schemars on hashes but nowhere else in the rust-bitcoin ecosystem did not prove that useful.

Remove schemars all together.

Fix: #3393

@github-actions github-actions bot added C-hashes PRs modifying the hashes crate doc labels Sep 20, 2024
@apoelstra
Copy link
Copy Markdown
Member

I'm not sure it should be a separate issue -- if we drop this test crate then we won't know when schemars eventually breaks. I think we should remove it all at once.

@tcharding tcharding force-pushed the 09-21-rm-schemars-test branch from 328cba7 to d52c56c Compare September 22, 2024 20:55
@github-actions github-actions bot added the test label Sep 22, 2024
@tcharding tcharding changed the title hashes: Remove schemars test crate Remove schemars all together Sep 22, 2024
@tcharding tcharding changed the title Remove schemars all together Remove schemars all together Sep 22, 2024
We introduced schemars as a personal favor to a user, and it broke our
CI repeatedly but eventually it seemed like it was stable (mainly, our
MSRV caught up with its MSRV) so we just let it slide. In the end having
schemars on hashes but nowhere else in the rust-bitcoin ecosystem did
not prove that useful.

Remove schemars all together.

Fix: rust-bitcoin#3393
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 58704c2 successfully ran local tests

@apoelstra
Copy link
Copy Markdown
Member

I will let my ACK sit for 24 hours before merging to give Kix a chance to complain -- but because this is breaking CI I'll be more agressive about one-ack merging it than I normally would for a "rip out a feature" PR.

@tcharding
Copy link
Copy Markdown
Member Author

FTR its an easy one to revert if needed.

@apoelstra apoelstra merged commit 40ba08f into rust-bitcoin:master Sep 24, 2024
@tcharding tcharding deleted the 09-21-rm-schemars-test branch October 14, 2024 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-hashes PRs modifying the hashes crate doc test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI on master is broken because of schemars again

2 participants