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

[WIP] Setup PR based release process #1407

Closed
wants to merge 18 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@zephraph
Copy link
Contributor

zephraph commented Oct 13, 2018

Fixes #1095

This PR will fundamentally rework how our deployment process happens.

The changes are as follows:

  • Changelog entries will be required
  • The release process will no longer use semantic-release
  • Releases will effectively happen on every PR merge*
  • The PR must include a label that describes the type of change
    • release:patch
    • release:minor
    • release:major
    • release:none

Remaining work:

  • Ensure changelog entries are required before merging (peril may already handle this)
  • Publish releases to github
  • Replace semantic release command (semantic release will be removed in a follow up PR once we've verified all functionality)
  • Setup CircleCI config to support a git push
  • Cover corner case where git push succeeds but npm publish fails
    • In this case the changelog will be updated with a version that may never show up in the releases. We'll need to check the tags and if the latest tag is of a lower version than the changelog then we'll want to group those changelog changes in with the master section and continue with the changelog release.

Follow up work:

  • Remove semantic-release
  • Pull release process out into a separate package
  • Allow changelog entries in the PR body

*Note: Due to timing issues w/ pull requests it's possible that multiple PRs will be grouped together in one release with the one of the largest change determining the version

@zephraph zephraph requested review from sweir27 , orta and alloy Oct 13, 2018

@zephraph zephraph changed the title Setup PR based release process [WIP] Setup PR based release process Oct 13, 2018

@damassi

This comment has been minimized.

Copy link
Member

damassi commented Oct 13, 2018

@zephraph - Two qs:

The PR must include a label that describes the type of change

How should one think about PRs that are unrelated to @artsy/reaction stuff? (tooling, internals, etc)

@artsyit

This comment has been minimized.

Copy link
Contributor

artsyit commented Oct 13, 2018

Deploy preview for artsy-reaction ready!

Built with commit 07b2133

https://deploy-preview-1407--artsy-reaction.netlify.com

@zephraph

This comment has been minimized.

Copy link
Contributor Author

zephraph commented Oct 13, 2018

@damassi I was talking to @orta today about potentially having a release:none tag or something. If it truly didn't change the source then we probably wouldn't want to trigger a release for it.

With that being said a lot of times making a tooling change does effect the source, in which case we should always use standard semantic versioning (IMO).

@damassi

This comment has been minimized.

Copy link
Member

damassi commented Oct 13, 2018

That makes sense 👍

Another q:

Why migrate away from semantic-release to something that's custom? There are lots of contributors and we're also following this pattern on other repos. Why not leverage command-line usage and hook into labels? I'm a little wary of roll-your-own solutions when there are many solid options in the OSS community.

@artsy artsy deleted a comment from artsyit Oct 13, 2018

@zephraph

This comment has been minimized.

Copy link
Contributor Author

zephraph commented Oct 13, 2018

We don't have to roll our own solution for actually publishing the package. I'm fine with using a tool for that, but semantic-release covers an entirely different usecase. The point of this PR is to essentially trigger releases on every PR (or most every PR) without having to write a custom commit message. Every time master CI runs, it'll check if it's the latest release (if it's tagged). If it's not, it'll look for all changed PRs since the last release, gather up their changelog entries, update the changelog, update the version, commit all that and tag the commit w/ the version. When CI runs on a commit w/ the latest tag it'll automatically deploy (or try to deploy) that version.

It's not really too crazy of a process and @orta has largely implemented most of the individual parts w/ danger. It's all about stitching things together. When this is done though, there should be very little custom code.

@zephraph

This comment has been minimized.

Copy link
Contributor Author

zephraph commented Oct 13, 2018

Let me get this first pass done. If you still have concerns, I'm more than willing to try to do it differently. Just trying to make this process as easy as possible. Definitely don't want to roll too much of my own stuff here 😅 .

@damassi

This comment has been minimized.

Copy link
Member

damassi commented Oct 13, 2018

Cool cool, lets be sure to keep semantic-release's plugin architecture in mind which can interface with things like conventional-changelog so a lot of work can be automated away.

@sweir27
Copy link
Contributor

sweir27 left a comment

Nice work-- I'm so so excited about this. Am curious what it looks like to include a changelog entry in the PR body? Would be good to have an example once this is out.

Also, will there be a default release-type if none is specified?

@alloy

This comment has been minimized.

Copy link
Member

alloy commented Oct 25, 2018

Seems like I didn’t actually post my comment previously. I like it! The one question I had basically echoes @sweir27’s question for an example.

@zephraph

This comment has been minimized.

Copy link
Contributor Author

zephraph commented Oct 25, 2018

Good questions @sweir27! The changelog entry in the PR isn't a necessary feature for this, but I thought it could be nice. I was thinking it could look something like the following...

## Changes
- Added a release process that triggers on PR labels
- Added the ability to specify changelog entries through PR bodies

Then after the PR is merged, the initial master build grabs the PR (already does this to get the label) and can commit and update to the changelog. Ultimately though, as I said, it isn't necessarily required. We could just use a danger rule to enforce changelog updates.

As for the default release type I was planning on adding a danger rule that fails CI if there isn't a label added. We could specify a default and we'd talked about that in the RFC. I'm just worried people would forget to check the label.

export const writeChangelog = (
changelogMarkdown: string,
changelogPath = CHANGELOG_PATH
) => write(changelogPath, changelogMarkdown)

This comment has been minimized.

@damassi

damassi Nov 29, 2018

Member

Nice and simple

@damassi

This comment has been minimized.

Copy link
Member

damassi commented Dec 3, 2018

@zephraph - would you mind turning the description list items into a check-list for tracking what has been completed? Makes it slightly easier to review as parts come in.

Fixed labels, bump package version, publish package?
Signed-off-by: Sarah Weir <sweir27@gmail.com>

@zephraph zephraph force-pushed the pr-releases branch from f306ed4 to a4e844e Dec 4, 2018

@damassi

This comment has been minimized.

Copy link
Member

damassi commented Jan 1, 2019

@zephraph - looks like this is what we're looking for? https://github.com/intuit/auto-release

@zephraph zephraph referenced this pull request Jan 3, 2019

Merged

Add auto-release #1779

@zephraph zephraph closed this Jan 3, 2019

@zephraph zephraph deleted the pr-releases branch Jan 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment