Skip to content

Forward-port check-semver scripts and seal WorkExt - #6548

Merged
apoelstra merged 2 commits into
rust-bitcoin:masterfrom
nyonson:check-semver-one-master
Jul 31, 2026
Merged

Forward-port check-semver scripts and seal WorkExt#6548
apoelstra merged 2 commits into
rust-bitcoin:masterfrom
nyonson:check-semver-one-master

Conversation

@nyonson

@nyonson nyonson commented Jul 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Forward-port of #6536, with some shell style cleanup.

@nyonson nyonson changed the title Check semver one master Forward-port check-semver scripts Jul 14, 2026
@nyonson
nyonson force-pushed the check-semver-one-master branch from 45a1b1e to 802005f Compare July 14, 2026 20:12
@github-actions github-actions Bot added ci C-bitcoin PRs modifying the bitcoin crate C-hashes PRs modifying the hashes crate C-units PRs modifying the units crate C-io PRs modifying the io crate C-primitives C-crypto C-p2p C-key_expression labels Jul 14, 2026
@nyonson nyonson changed the title Forward-port check-semver scripts Forward-port check-semver scripts and seal WorkExt Jul 14, 2026
@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown

🚨 API BREAKING CHANGE DETECTED

To see the changes click details on "Continuous integration / Check (api)" job then expand "Run api" 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 Jul 14, 2026
@nyonson
nyonson force-pushed the check-semver-one-master branch from 802005f to d20634a Compare July 14, 2026 20:29
@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown

🚨 API BREAKING CHANGE DETECTED

To see the changes click details on "Continuous integration / Check (api)" job then expand "Run api" and scroll to the end of the section.

tcharding
tcharding approved these changes Jul 14, 2026
View reviewed changes

@tcharding tcharding left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ACK d20634a

@nyonson
nyonson force-pushed the check-semver-one-master branch from d20634a to a1b05dc Compare July 24, 2026 16:16
@github-actions github-actions Bot removed C-hashes PRs modifying the hashes crate C-units PRs modifying the units crate C-io PRs modifying the io crate C-primitives C-crypto C-p2p C-key_expression labels Jul 24, 2026
@nyonson
nyonson force-pushed the check-semver-one-master branch from a1b05dc to dff185b Compare July 24, 2026 17:40
@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown

🚨 API BREAKING CHANGE DETECTED

To see the changes click details on "Continuous integration / Check (api)" job then expand "Run api" and scroll to the end of the section.

@nyonson
nyonson marked this pull request as ready for review July 24, 2026 17:47
@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown

🚨 API BREAKING CHANGE DETECTED

To see the changes click details on "Continuous integration / Check (api)" job then expand "Run api" and scroll to the end of the section.

@nyonson

nyonson commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

bf7e391: up to date forward port of the new script, and added a commit for the higher shellcheck standards here on master.

@nyonson
bitcoin: seal WorkExt
73efa99 
An un-sealed WorkExt exposes a non-additive feature. With the `std`
feature is enabled, a function is added without a default implementation.
Sealing the trait since it is not designed for consumers to implement.
This also makes it additive.
@nyonson
nyonson force-pushed the check-semver-one-master branch from bf7e391 to 2f420f8 Compare July 26, 2026 21:53
@github-actions

github-actions Bot commented Jul 26, 2026

Copy link
Copy Markdown

🚨 API BREAKING CHANGE DETECTED

To see the changes click details on "Continuous integration / Check (api)" job then expand "Run api" and scroll to the end of the section.

@nyonson

nyonson commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

2f420f8: rebase for API file fixes

@tcharding

tcharding commented Jul 27, 2026

Copy link
Copy Markdown
Member

I'll ack. It reads a bit weirdly to me to use main function but define it in the middle of the file. I.e helper functions at the top for no real reason. But anyways ....

tcharding
tcharding approved these changes Jul 27, 2026
View reviewed changes

@tcharding tcharding left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ACK 2f420f8

@tcharding

tcharding commented Jul 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

I didn't read the shell all that closely since its a port.

@nyonson

nyonson commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

I'll ack. It reads a bit weirdly to me to use main function but define it in the middle of the file. I.e helper functions at the top for no real reason. But anyways ....

I can fix this up. I ain't the greatest shell hacker.

@nyonson
ci: merge check-semver scripts into one
2be8875 
Clean up a lot of the handling by leveraging check-semver features. This
allows the script to handle the semver trick.
@nyonson
nyonson force-pushed the check-semver-one-master branch from 2f420f8 to 2be8875 Compare July 28, 2026 22:44
@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown

🚨 API BREAKING CHANGE DETECTED

To see the changes click details on "Continuous integration / Check (api)" job then expand "Run api" and scroll to the end of the section.

@nyonson

nyonson commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

2be8875: re-order main function, also fix the bash array initialization

@tcharding

tcharding commented Jul 29, 2026

Copy link
Copy Markdown
Member

hehe you put the main function at the bottom, it goes at the top ya goose :)

If you want, take a look at contrib/check-error-reexports.sh. I copied that style from the original rustup.sh. If you've not read that I'd highly recommend it, its funny AF. (eg # Don't make typos. You just have to be better than that.)

tcharding
tcharding approved these changes Jul 29, 2026
View reviewed changes

@tcharding tcharding left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ACK 2be8875

apoelstra
apoelstra approved these changes Jul 31, 2026
View reviewed changes

@apoelstra apoelstra left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ACK 2be8875; successfully ran local tests

@nyonson

nyonson commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

hehe you put the main function at the bottom, it goes at the top ya goose :)

If you want, take a look at contrib/check-error-reexports.sh. I copied that style from the original rustup.sh. If you've not read that I'd highly recommend it, its funny AF. (eg # Don't make typos. You just have to be better than that.)

That is awesome. I don't have strong feelings on it, but it does seem like the "where does the main function live" convention isn't very strong? Been trying to read up on general best shell practices and it seems all over the place.

@apoelstra
apoelstra merged commit 49c2c20 into rust-bitcoin:master Jul 31, 2026
25 checks passed
@tcharding

tcharding commented Aug 3, 2026

Copy link
Copy Markdown
Member

Obviously my subjective opinion is the most correct and I rekon important stuff goes at the top of the file. 'Helpers' and less import stuff below. Andrew is pleasantly ambivalent on such triviality so in rust-bitcoin I get to push most of these ideas through ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@apoelstra apoelstra apoelstra approved these changes
@tcharding tcharding tcharding approved these changes

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@nyonson @tcharding @apoelstra