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

Release CI: Bulletproof SemVer Checks #4817

Open
ggwpez opened this issue Jun 18, 2024 · 7 comments
Open

Release CI: Bulletproof SemVer Checks #4817

ggwpez opened this issue Jun 18, 2024 · 7 comments

Comments

@ggwpez
Copy link
Member

ggwpez commented Jun 18, 2024

The current CI checks are structured in a way that the R0-silent label skips the SemVer checks. This is good in general, but i already saw two merge requests that declared R0 and had accidental breaking changes.

I think we should do something like this:

graph TD
    A(Merge Request) --> B{SemVer Change Detected?}
    B -- No --> Advise{Targets Stable?}
    Advise -- No --> Ok
    Advise -- Yes --> Comm[Comment Generic backport advise]
    B -- Yes --> R1{Has R0 label?}
    R1 -- No --> S{Targets Stable?}
    S -- No --> Ok2[Check PrDocs]
    S -- Yes --> nM[Ensure PrDocs are minor]
    R1 -- Yes --> Major{Is Major change?}
    Major -- No --> W[Warn about removing R0]
    Major -- Yes --> Stable{Targets Stable?}
    Stable -- No --> W
    Stable -- Yes --> E[Error: Forbidden]
Loading
@obi1kenobi
Copy link

👋 cargo-semver-checks maintainer here, quick note that cargo-semver-checks will never report breaking changes if it detects the version bump is major, since they are all allowed.

So unless you have some peculiar config or arrangement of the workflow, my guess is that the "is major change" decision point in the workflow will always evaluate to "no" as currently diagrammed.

@ggwpez
Copy link
Member Author

ggwpez commented Jul 14, 2024

Hey @obi1kenobi, thanks for chiming in so proactively 😄

We are currently using the library of the cargo semver check in the parity-publish tool. My guess here is that we would add a sub-command to scan all the changes for any major change and then error. We already have a --max-bump argument on one of our commands here. I assume this new subcommand would do it somehow similar.

That should work, or?

@obi1kenobi
Copy link

Oh nice, TIL that you are consumers of the library API! Not too many users like that yet, so it's nice to see it's useful for someone :)

Ultimately, it depends on the precise details of how you generate the rustdoc JSON — it might or might not include the crate version from which it was generated, so cargo-semver-checks might or might not be able to act on those versions. You could make it work, but it might be a bit brittle so I would keep an eye on it.

As an aside, if you find cargo-semver-checks to be a valuable part of your workflows, I'd really appreciate if Parity Technologies would consider becoming a sponsor! cargo-semver-checks could be 100x better than it is today (for example, here are some concrete ideas) but it's being held back by a lack of sustainable funding. I'd like to be a full-time open-source maintainer, and I need users' help to make that happen!

@bkchr
Copy link
Member

bkchr commented Jul 15, 2024

@obi1kenobi would you be open to get paid in crypto? E.g. USDT or DOT?

@obi1kenobi
Copy link

I'd most prefer fiat currency through GitHub Sponsors. That automatically handles tax considerations for me, minimizing paperwork and bureaucracy I have to deal with, and is also automatically visible as income to my bank which makes it easier to be eligible for loans etc.

If you have a strong preference against fiat, I could try to figure out the tax paperwork related to accepting crypto. But I'm worried about slippage and transaction costs eating a large chunk of the value when I need to convert it to fiat to pay rent and bills.

@bkchr
Copy link
Member

bkchr commented Jul 15, 2024

USDT is for example a stable coin. Aka there is only the volatility of USD.

@obi1kenobi
Copy link

If it's all the same on your end, I would prefer Ethereum-based (ERC-20) USDC over USDT for reasons of familiarity and convenience, so I can spend maximum time coding.

If it's a big hassle on your end to offer USDC, then I could probably figure out how to make USDT work as well.

I really appreciate you looking into this!

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

No branches or pull requests

3 participants