getsentry / craft

The universal Sentry release CLI πŸš€
MIT License
133 stars 15 forks source link

Add config for latest tagging branches #498

Closed mydea closed 1 year ago

mydea commented 1 year ago

Problem

When publishing backported versions, they are auto-set as latest in various targets (npm, github, possibly others). This means that you either need to take manual steps or have to follow up with a release of the "actual" latest, which is far from ideal.

For npm, you can circumenvent this by putting something like this on the backported branchs package.json:

"publishConfig": {
  "tag": "v1"
}

For github, there is no way to do that (and e.g. Homebrew depends on this)

Proposed solution

We propose to add a new top level config to allow to opt-in to define a list of target branches that should get the latest tag:

# .craft.yml
minVersion: '0.23.1'
changelogPolicy: simple
latestTargetBranches: 
  - master

If this is configured, craft targets should only add the latest tag (or similar behavior, depending on the target) when merging into one of the defined branches.

With this, projects can opt-in to only release as latest when merging into e.g. master/main branch, and when merging into e.g. a 1.x branch we can ensure to not mark this release as latest.

BYK commented 1 year ago

Craft already knows about the "default" branch so it feels more appropriate to make this the default behavior?

For backwards compatibility, you can internally enable this only for config files setting a minVersion above the change date.

mydea commented 1 year ago

Sounds good to me too!

So basically you mean:

mydea commented 1 year ago

Actually, we just thought about this and this will not necessarily work - as the default branch is not necessarily the branch where you run releases from.

Concretely, in sentry-javascript the default branch is develop, but we release from/to the master branch (we use gitflow and still merge into develop while a release is pending etc). So for cases like these you'd still need to configure this.

We can still do both I guess, do this by default for the default branch, but allow to configure/overwrite this if needed?

BYK commented 1 year ago

Concretely, in sentry-javascript the default branch is develop, but we release from/to the master branch (we use gitflow and still merge into develop while a release is pending etc). So for cases like these you'd still need to configure this.

Ah, that's quite unfortunate (and unexpected for me) πŸ˜…

We can still do both I guess, do this by default for the default branch, but allow to configure/overwrite this if needed?

I'd say yes but I feel like it may complicate things unnecessarily if you were looking to get this out of the door quickly. Use your own judgement then :) For the given scenario, your initial proposal makes sense to me and easy and harmless enough to start with.

mydea commented 1 year ago

At the end, we ended up implementing this different, per-target. Some targets already do this (e.g. registry, aws lambda layer, ...). So I added this to the github target as well (https://github.com/getsentry/craft/pull/503), and added a new config for the npm target to also be able to do this (https://github.com/getsentry/craft/pull/504).