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

Continuous delivery #840

Closed
mightyiam opened this issue Jun 2, 2022 · 18 comments · Fixed by #909
Closed

Continuous delivery #840

mightyiam opened this issue Jun 2, 2022 · 18 comments · Fixed by #909

Comments

@mightyiam
Copy link
Owner

One of the causes for burnout is repetitive tasks that should be automated away. I made too many manual releases for this project. The next release coming out of this project as far as I'm concerned will be triggered by a workflow on push/merge to the master branch.

@fcole90
Copy link

fcole90 commented Jun 13, 2022

Do you already have a PR for this? How can one best contribute? 😊

@mightyiam
Copy link
Owner Author

@fcole90 me and @rostislav-simonik are pairing on a weekly schedule on this. If the schedule happens to fit you, we could mob. I have a lot of experience mob programming, as I am the founder of @mobusoperandi.

@mightyiam
Copy link
Owner Author

@voxpelli I would like to decouple as much as possible from platform specific features. Especially proprietary ones. Using that release-please GitHub action would mean exactly that.

@voxpelli
Copy link

@mightyiam Which "platform specific" features are you talking about here?

Seems to me like both semantic-release and release-please-action are implemented through GitHub Action workflows?

Neither are proprietary and neither needs any app to be installed to the organization?

@mightyiam
Copy link
Owner Author

The platform specific features I'm referring to are GitHub Actions. That release-please-action is a GitHub Actions action. semantic-release is not. If we were to migrate to another CI provider, it would be less work had we chosen semantic-release. This is classic vendor lock-in avoidance.

@voxpelli
Copy link

voxpelli commented Jun 15, 2022

Both are based on conventional commit formatted commit messages, so swapping them out for something else is very simple.

And - release-please isn’t a GH Action at its core: https://github.com/googleapis/release-please

It’s only a vendor lock-in if we can’t easily move away from it. That’s easy with both.

@mightyiam
Copy link
Owner Author

CD is mostly implemented. Just missing a GitHub app for the purpose of bypassing required PR branch protection. I've done this twice before in different projects. I require the greater permission (owner?) on the @standard organization to create and install the app. @standard/team if anyone can give me the permission, please...

@voxpelli
Copy link

@mightyiam Can you link to what CD setup you have opted for? And what app it is that you need?

@mightyiam
Copy link
Owner Author

@mightyiam Can you link to what CD setup you have opted for? And what app it is that you need?

There's a branch. Probably mob/cd.

The app is a GitHub App that can be created via GitHub's web UI. Not much to it, really.

@voxpelli
Copy link

voxpelli commented Jun 22, 2022

Right, so what you want to do is this? https://semantic-release.gitbook.io/semantic-release/recipes/ci-configurations/github-actions#pushing-package.json-changes-to-a-master-branch

And there they say:

Note: Automatically populated GITHUB_TOKEN cannot be used if branch protection is enabled for the target branch. It is not advised to mitigate this limitation by overriding an automatically populated GITHUB_TOKEN variable with a Personal Access Tokens, as it poses a security risk.

And you're right, mob/cd looks to be the correct branch. Here's a comparison link for others who are following along: https://github.com/standard/eslint-config-standard-with-typescript/compare/mob/cd

I would suggest making a draft PR of that branch and link that PR to this issue. That way we can talk more hands on about what needs to be done, which would be more helpful.

@voxpelli
Copy link

voxpelli commented Jun 23, 2022

An alternative solution as I see it is to use release-please-action as it rather than pushes to the protected branch opens a PR against it, which can then be reviewed and merged normally.

When such a PR is merged, release-please-action creates a release for the project, something which is outside branch protection rules, and it allows for publishing to npm as well.

@voxpelli
Copy link

I would also suggest looking into https://commitlint.js.org/ as that enables tools like https://github.com/vivaxy/vscode-conventional-commits to be used in authoring commits

@mightyiam
Copy link
Owner Author

The release-please workflow — I don't like it. It takes merging a PR to have a release. I'd like more automation, where I have to do nothing and releases just occur from pushes to master.

We already have commit linting in place. That's important for such a workflow.

Here's a project I'm involved with which has a CD setup similar to what I'm trying to achieve here.

The only thing I'm missing is the GitHub App created and installed and the app ID and token as secrets in the repo.

@voxpelli
Copy link

So: One release per push to master? So merging four PR:s will create four releases, not one with all of them?

That sounds quite noisy to me

Skärmavbild 2022-06-27 kl  11 12 58

@mightyiam
Copy link
Owner Author

Whether a release occurs depends on commit messages since last release. For example, typically, the types feat/fix/perf and possibly doc and anything with a BREAKING CHANGE: ... triggers a version bump while chore/test/build/refactor do not.

Automatic releases takes the drama away from them. Who cares how many releases are made as long as they all explain themselves with nice changelog entries?

Users don't typically complain about frequency of releases — they complain about frequency of breaking releases.

@meyfa
Copy link

meyfa commented Jun 27, 2022

It takes merging a PR to have a release.

Since whichever tool is chosen does not also review PRs, merging a PR will always be required for a release to happen. There's no way to get around some amount of manual work. In fact, the amount of work needed might be reduced with release-please since there's an additional safety net and hence less upfront work on each PR is required.

Users don't typically complain about frequency of releases — they complain about frequency of breaking releases.

A Renovate Bot config rule will have to be added for consumers of this package if releases happen so often, to cut down email spam through GitHub notifications. (Arguably still better than no releases, though.) Re number of breaking releases: release-please would allow bundling of multiple breaking changes into a single release, which seems difficult to achieve with semantic-release.

@voxpelli
Copy link

voxpelli commented Jun 27, 2022

Re number of breaking releases: release-please would allow bundling of multiple breaking changes into a single release, which seems difficult to achieve with semantic-release.

Indeed, and would enable manual pre-releases for such breaking changes, like we did with [email protected], [email protected] and [email protected]

I find that helpful in verifying releases in the wider ecosystem, enabling eg. myself to start trying them out in my own projects and verifying them there, before pushing the new major to the wider community.

@meyfa
Copy link

meyfa commented Jun 28, 2022

On the other hand, there actually exists a similar permissions/token problem with release-please. See https://docs.github.com/en/actions/using-workflows/triggering-a-workflow#triggering-a-workflow-from-a-workflow.
The repository's testing/linting CI will not run on the release PR. This means that, if PRs are set to be unmergeable until required jobs have passed, the release PR cannot ever be merged. The only two solutions are using a Personal Access Token for release-please, or disabling the "PR passes CI to be mergeable" requirement on the repo.

mightyiam added a commit that referenced this issue Aug 1, 2022
closes #840

Co-authored-by: Rostislav Simonik <[email protected]>
mightyiam added a commit that referenced this issue Aug 8, 2022
closes #840

Co-authored-by: Rostislav Simonik <[email protected]>
rostislav-simonik added a commit that referenced this issue Aug 8, 2022
closes #840

Co-authored-by: Rostislav Simonik <[email protected]>
mightyiam added a commit that referenced this issue Aug 8, 2022
closes #840

Co-authored-by: Rostislav Simonik <[email protected]>
rostislav-simonik added a commit that referenced this issue Aug 9, 2022
closes #840

Co-authored-by: Rostislav Simonik <[email protected]>
rostislav-simonik added a commit that referenced this issue Aug 9, 2022
closes #840

Co-authored-by: Rostislav Simonik <[email protected]>
eslint-config-love-release-bot bot pushed a commit that referenced this issue Sep 13, 2022
## [23.0.0](v22.0.0...v23.0.0) (2022-09-13)

### ⚠ BREAKING CHANGES

* add rules from @typescript-eslint: no-extra-parens,
no-loss-of-precision, object-curly-spacing, space-before-blocks,
comma-dangle.

### Features

* use all typescript-eslint equivalents ([1e8764d](1e8764d)), closes [#582](#582) [#583](#583)

### CI

* continuous delivery ([971c680](971c680)), closes [#840](#840)

### Build system / dependencies

* renovate automergeMinor github actions ([02dde9d](02dde9d)), closes [#910](#910)
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

Successfully merging a pull request may close this issue.

4 participants