feat: Allow to configure latestTargetBranch

[mydea]

This PR allows to add a new top-level config latestTargetBranch, which can be set to a branch name.

Only when merging into this branch name, will targets try to set the given release as "latest".

Going forward, if your minVersion > 1.8.0, the default behavior will be to use the default branch as default value for this. For older minVersion, the current behavior will remain (=every release will be tagged as latest).

I have implemented this for:

• npm
• github

A few targets try to do this automatically already (e.g. registry or awsLambdaLayer update latest.json based on the previous version number). We can also add this for further targets that do this later (I don't know how all of them work) - but this is very much additive so we're safe to ignore this were it's not supported yet (=behavior will just remain the same).

Closes #498

[asottile-sentry]

why can't this just be the default branch? that seems the most logical target and wouldn't need configuration

if we're going to go forward with this: this new feature needs to be implemented in all the relevant targets before merging

[mydea]

As discussed in the linked issue: #498 this does not work for e.g. sentry-javascript, where we use a GitFlow process with a develop & master branch. develop is the default branch but we release off of master.

I have implemented this for all targets that I know off that do that. If you or anybody else knows of other targets that set a latest tag or similar, let me know (or just feel free to implement them yourself). IMHO this is already very valuable for npm & github only, we've had concrete issues opened about this and it is a massive footgun to publish a backported version and be surprised by latest being incorrectly updated.

[asottile-sentry]

what benefit is gitflow getting you -- it sounds like it's most of the problem and isn't used elsewhere at the company

[mydea]

what benefit is gitflow getting you -- it sounds like it's most of the problem and isn't used elsewhere at the company

The JS monorepo is rather different from most other SDKs because we simply merge a lot. Since doing a release can take quite some time (for tests to pass etc. it can be 30min or more, and with feedback cycles we can have a pending release for hours, sometimes), we don't want to block any merging while a release is in process. This is also hard to enforce because so many different people contribute to the repo at the same time, and not all of them are in the core SDK team that are on top of publishing etc.

but also, I don't really see how this is relevant to this - craft should not enforce a git branching process, I'd say.

Also, this PR is specifically built so that this will use the default branch by default, it's just an additonal config that can be added if you do not release off of your default branch. So for the "most common" scenario you do not need to configure anything.

[asottile-sentry]

I'm not convinced we need this at all -- the registry target seems to handle this correctly without the complexity (it knows when the release is non-latest and does not update latest)

[asottile-sentry]

I also don't think the sdk monorepo receives more merges than sentry for example making the claim that an alternative branching strategy is needed dubious. yes craft probably shouldn't care that much -- but sentry should standardize on one development flow rather than having it be special per repository.

[Lms24]

Looks good to me, thanks for taking care of this!

[Lms24]

q: I guess we need to set some tag so that NPM doesn't default to latest?

[lforst]

@asottile-sentry Before we switched our release process to gitflow, our releases sometimes took upwards of 3 hours because accidental pushes to the main branch invalidated them because craft requires the release branch to be up to date. Asking people not to merge because a release is pending did not work for organizational reasons. We have multiple teams merging into sentry-javascript. We switched to gitflow out of necessity because craft screwed us.

Frankly, I am genuinely interested in the concrete reason why you're blocking this. I hope it's not because you're worried that you'll be pinged about issues with craft - cause more than you or anybody else, we're ourselves affected by it. We are merely trying to solve issues here that have been plaguing us for years. Just today, I had to do a double release again because craft doesn't support releasing old versions.

[asottile-sentry]

my main concern is bolting on more complexity without tests to a tool which is under-tested and under-supported. there are simpler ways to accomplish what you need to solve your problems with the existing interfaces and I'd strongly prefer fixing those rather than adding yet-another-untested option.

[lforst]

there are simpler ways to accomplish what you need to solve your problems with the existing interfaces

Can you point these out?

[asottile-sentry]

there are simpler ways to accomplish what you need to solve your problems with the existing interfaces

Can you point these out?

registry manages to detect when a version is non-latest and avoids updating symlinks -- we can do the same for npm / github

[mydea]

registry manages to detect when a version is non-latest and avoids updating symlinks -- we can do the same for npm / github

this is not that easy because our current flow for npm simply does not know what package name we are even publishing. We only know the version & the path to a tarball. We'd need to unpack all tarballs, read the package.json from the file, then make a request to npm via e.g. npm view {pkgName} version in order to be able to compare the versions. If you think this is less complexity/easier to do, I'm also happy to implement it this way.

For Github, we need to fetch the latest release, which should hopefully be possible via octokit (*but haven't checked yet).

[asottile-sentry]

registry manages to detect when a version is non-latest and avoids updating symlinks -- we can do the same for npm / github

this is not that easy because our current flow for npm simply does not know what package name we are even publishing. We only know the version & the path to a tarball. We'd need to unpack all tarballs, read the package.json from the file, then make a request to npm via e.g. npm view {pkgName} version in order to be able to compare the versions. If you think this is less complexity/easier to do, I'm also happy to implement it this way.

For Github, we need to fetch the latest release, which should hopefully be possible via octokit (*but haven't checked yet).

ah I see -- that is a little more complicated than I thought it was. registry has the advantage that it can see the entire version tree all on its own.

let's pursue the current approach then -- but please write some tests which demonstrate this feature

[mydea]

Based on your feedback, I ended up splitting this up in two PRs and instead tried to implement the latest tagging in the github & npm targets specifically:

• feat(npm): Allow to configure checkPackageName for npm target #504
• feat(github): Only set latest when new version > old version #503

For github, that should work easily enough. For NPM I introduced a new config option for npm specifically, as that is the main thing that is tricky to infer, sadly.