ci: automatic releases

[sparten11740]

Automates releases by releasing on merge to master for relevant commit types (feat, fix, perf, and breaking changes).

If we want to use @semantic-release/git, which we should, we'll have to use a PAT of the machine account and empower that machine account to bypass branch protection rules. The plugin is responsible for updating the version in package.json and adding the CHANGELOG.md! Decided to use @semantic-release/github instead for tighter security so we don't have to allow the machine user to bypass the branch protection rule. This will result in stale package versions in git, but still have the correct version in the published package.

Todos:

• create a NPM_PUBLISH_TOKEN secret that has an NPM automation token for the account that will get exclusive access to @exodus/schemasafe

Testplan

A first sanity check of the configuration can be done using the dry run mode locally.

1. Checkout this PR
2. Apply the following diff

Subject: [PATCH] add test branch
---
Index: .releaserc.json
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/.releaserc.json b/.releaserc.json
--- a/.releaserc.json	(revision f9059cf10c831fad0f2a0fc8d17ab1d1c6fc8a85)
+++ b/.releaserc.json	(date 1699578894912)
@@ -1,5 +1,5 @@
 {
-  "branches": ["master"],
+  "branches": ["master", "sparten11740/ci/automatic-releases"],
   "plugins": [
     [
       "@semantic-release/commit-analyzer",

3. run yarn release -d. verify echo $? returns 0 and you see some nice release notes in the stdout of the first command

This doesn't test the full integration such as correctly configured PAT's and branch protection rules. We'll only be able to test these when this PR gets merged

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f28dd6f) 99.54% compared to head (20ec800) 99.54%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

[olistic]

@ChALkeR Can you please review?

[mvayngrib]

tACK (let's wait for @ChALkeR before merging)

[sparten11740]

@RyanZim we need an npm automation token with access to @exodus/schemasafe saved as NPM_PUBLISH_TOKEN secret. Is that something you could look into?

We also still need a GH_AUTOMATION_PAT for the exodus-github-robot and allow exodus-github-robot to bypass the branch protection rule, so it can commit release assets

I can't do any of this and I also can't look at the repo configuration since I don't have admin access

[ChALkeR]

why is this needed?

[sparten11740]

we are installing dev dependencies here as well and semantic-release requires node >= 18, so this is required to silence yarn about the engine mismatch

[ChALkeR]

I'm unsure why exactly this would be helpful.

The main chores of this lib are not releases per se, but logic fixes, and autorelease functionality adds some amount of complexity.

Which could be more than the release complexity it reduces.

A more helpful setup would be to always (when actual) have an PR which updates json-schema-test-suite gitmodule to latest)

[RyanZim]

@ChALkeR the intent is to reduce the number of people with publish access here; see #165 for context.

[ChALkeR]

Blocking visibly until the exact mechanism is figured out that doesn't degrade security.

[ChALkeR]

#165 is understandable and should be fixed, but this approach just makes things worse at the current state (explained via other channels). It can make things better if configured properly though.

[sparten11740]

It can make things better if configured properly though.

the whole premise of this PR is to have an adequate branch protection rule. I don't have admin access and cannot see what's currently configured. You will have to take the lead here, but some suggestions:

• Require a pull request before merging
• Require 2 approvals
• Dismiss stale pull request approvals when new commits are pushed
• Require review from Code Owners
• Require conversation resolution before merging
• Require status checks to pass
• Require signed commits

[ChALkeR]

@sparten11740 Yeah, a proper branch configuration and removing npm users write access first would unblock this.

[ChALkeR]

let's file a separate PR for this (and bump that to 20)

[sparten11740]

it's either this or running yarn install with --ignore-engines. I prefer the node version update

[sparten11740]

changed to 20

[sparten11740]

we are actually getting test failures here in node 20, reverted back to 18

[ChALkeR]

let's do this in a separate PR, unclear what the differences are yet and this is an orthogonal change

[ChALkeR]

see above comments
perhaps we should create a more protected release branch and publish from it? Making releases separate prs to the release branch and require them to have more than 2 reviews

[sparten11740]

Perhaps we can lower the requirement to merge to master and instead make the requirements for releases stricter?

@sparten11740 @mvayngrib thoughts?

I don't see much of an advantage in that, the amount of time for the changes to get released would probably be the same, if not even more. The reviewing effort increases and we potentially have to backport fixes from issues found in the release PRs.

[sparten11740]

@RyanZim can you create a new environment named release, restrict the environment to be used from master, add the npm token as environment secret NPM_PUBLISH_TOKEN to that environment, and remove the previous repository secret with the same name?

[sparten11740]

can you create a new environment named release, restrict the environment to be used from master, add the npm token as environment secret NPM_PUBLISH_TOKEN to that environment, and remove the previous repository secret with the same name?

@ChALkeR, Ryan is not admin. Would you mind doing this? He'll keybase the token

[mvayngrib]

for now, i vote we keep the merge policy for master strict and do auto-releases and revisit this later. Otherwise we’ll have to review release PRs, which is extra pain (plus we’ll have to re-review everything merged since the previous release if we don’t trust master anymore)

the DX pain on master only exists when there are multiple PRs open at the same time. We can potentially improve this in the future as follows (discussed in DM with @sparten11740):

1. make sure PR checks run on the merge commit with master and not on the PR head (supposedly taken care of by the latest actions/checkout action)
2. invalidate + re-run checks on all open PRs when any PR is merged (because it changes the base for all of those PRs. h/t @sparten11740 for pointing this out)
3. use a custom action instead of GH’s settings to enforce 2 reviews pre-merge to master, and don’t “dismiss” reviews as stale if the diff with the merge base doesn’t change after rebase

the above setup could be useful for any of our repos, but isn’t a high priority atm

[RyanZim]

can you create a new environment named release, restrict the environment to be used from master, add the npm token as environment secret NPM_PUBLISH_TOKEN to that environment, and remove the previous repository secret with the same name?

Done

[mvayngrib]

what's the plan here?