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

chore: set up auto for versioning/release management #856

Merged
merged 9 commits into from
Jul 22, 2021

Conversation

hydrosquall
Copy link
Member

@hydrosquall hydrosquall commented Jul 16, 2021

Motivation

Changes

  • Adds + configures auto . This way, you get hinting/docs if editing locally.
  • Renames the job, otherwise it doesn't run by default since I wasn't a maintainer when I first opened the PR
  • Does not remove semantic-release or related config until we confirm this is working. (It's ok)
  • Ran auto create-labels locally to seed repository with library labels
    Image 2021-07-15 at 9 10 27 PM link

Testing

Notes on comparison vs semantic-release

  • auto supports opening per-PR canary deploys rather than depending on merges to a release branch
  • Philosophically, auto is encouraging releasing all the time (even non-semantic releases)
📦 Published PR as canary version: 0.94.2--canary.856.e019c4a.0

✨ Test out this PR locally via:

npm install ts-json-schema-generator@0.94.2--canary.856.e019c4a.0
# or 
yarn add ts-json-schema-generator@0.94.2--canary.856.e019c4a.0

@hydrosquall hydrosquall added release Create a release when this pr is merged internal dependencies Update one or more dependencies version labels Jul 16, 2021
.autorc.json Outdated
"first-time-contributor",
"released"
],
"onlyPublishWithReleaseLabel": true
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@domoritz , if we remove this boolean, the library will auto-publish on every merge to master. I suggest we don't enable that until you're first happy with how the auto-version incrementing logic works (it depends on committers adhering well to git commit message guidelines).

There are other options for setting version (setting a major/minor/patch github label), or asking users to check off the appropriate checkbox in their PR description when opening the PR, but this is probably the easiest place to start.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering about all the labels that this process added. I am pretty strict with enforcing correct pull request descriptions withs semantic commits. Can we use them to automatically set the right version? Or is the idea of the labels that they get added automatically?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use them to automatically set the right version?

I wasn't sure about the semantic commit situation in this repo because I didn't see commit-hooks. However, I think I saw some linting config in a different vega repo. Whether having good commit messages is checked by eye or tool, nteract/data-explorer#66, having manual labeling as an option is just a fallback for inaccurate messages.

The automatic labeling is based on conventional-commits, which most likely has the same behavior as the semantic-release plugin.

https://intuit.github.io/auto/docs/generated/conventional-commits

If you wanted to do the manual checkbox thing, it could be achieved with this plugin:

https://intuit.github.io/auto/docs/generated/pr-body-labels

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Last link: if any projects have more specific labeling needs than the defaults (or different mappings in terms of what we consider major/minor), they can be customized by passing in a custom array in the config:

https://intuit.github.io/auto/docs/configuration/autorc#labels

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is how we ensure commit messages:

Screen Shot 2021-07-15 at 21 34 53

I don't want additional labels if they are not necessary.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh neat I missed the semantic pull request integration! That's even more reliable than a commit hook since it doesn't depend on someone setting up local config properly. I might set that up for another project, thanks for the tip.

Regarding labels: I think we can delete the unused tags then. The other ones are optional and by default would help organize the auto-generated changelog, but are not required. I just deleted most of the added tags, with the exception of release and released since those are important for keeping track of what has been deployed.

@domoritz
Copy link
Member

Btw, I'm happy to remove the semantic-release dependency in this pull request as well.

@domoritz
Copy link
Member

How do canary releases work if the pull request comes from external (as most prs do)? I suppose they won't happen in that case.

@hydrosquall
Copy link
Member Author

hydrosquall commented Jul 16, 2021

Btw, I'm happy to remove the semantic-release dependency in this pull request as well.

Cleaned in e019c4a , undoes 18f6ca7 + d6e8e35

How do canary releases work if the pull request comes from external (as most prs do)? I suppose they won't happen in that case.

I think they won't run if the external person is a 1st time contributor, per https://github.blog/changelog/2021-04-22-github-actions-maintainers-must-approve-first-time-contributor-workflow-runs/, but after an initial approval, I think newcomers would have the ability to publish canaries.

@hydrosquall hydrosquall changed the title chore: set up auto for automated release management chore: set up auto for versioning/release management Jul 16, 2021
"assets": [
"package.json"
],
"message": "chore(release): ${nextRelease.version}"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Version bumps by default look like this : nteract/data-explorer@252d43a

@domoritz
Copy link
Member

but after an initial approval, I think newcomers would have the ability to publish canaries.

Secrets are not exposed to forks so not npm releases for them.

Screen Shot 2021-07-15 at 21 37 23

@hydrosquall
Copy link
Member Author

but after an initial approval, I think newcomers would have the ability to publish canaries.

Secrets are not exposed to forks so not npm releases for them.

Screen Shot 2021-07-15 at 21 37 23

Ah, good to know. I guess the primary initial beneficiaries of canary builds would be project maintainers. I think this limitation makes sense as a way to reduce the risk of abuse (e.g. people spamming your NPM account with bad builds), but we can still help give people a fast feedback loop if we decide to treat master as a pre-release branch (so it publishes everything using the command next), and make a new branch like stable or similar for "official" releases.

Let me know if you'd like to pursue that option (adding a new branch for stable, and treating master as a source of pre-releases) in this PR, or save that for a future one.

@domoritz
Copy link
Member

Nah, I want master to be the branch everything goes into and make releases from time to time.

So, if we accept this pull request, what happens to prs from forks? Would the ci fail since the NPM token and GitHub token don't exist?

How do I make a release now? I used to call a particular GitHub action to kick off a release. How do I make a release after all prs are merged? Or does a release always happen with a pull request?

@hydrosquall
Copy link
Member Author

hydrosquall commented Jul 16, 2021

So, if we accept this pull request, what happens to prs from forks? Would the ci fail since the NPM token and GitHub token don't exist?

Good point. We have at least 2 options

  1. Check for presence of the environment variable, quit job early if not present
  2. Quit if the repository context isn't vega//ts-json-schema-generator https://github.community/t/stop-github-actions-running-on-a-fork/17965/2 .

Let me know which one you prefer.

How do I make a release now? I used to call a particular GitHub action to kick off a release. How do I make a release after all prs are merged? Or does a release always happen with a pull request?

As it's currently set up, the workflow will release every merged PR, since the action is triggered on push, but it could be changed. If you'd prefer to manually make releases, we'd assign a different trigger to the github action (like dispatch_workflow).

You can check to confirm you like what the command would do on the main branch by running yarn run auto latest --dry-run (https://intuit.github.io/auto/docs/generated/shipit). Currently since we're running on a feature branch, we're seeing the results of yarn run auto canary.

@domoritz
Copy link
Member

Let me rephrase. I am okay with canary releases with every pull request but I don't want releases where I have to promise backwards compatibility with every merged pull request. I want to manually decide when we make a release. Bonus points if I don't have to do anything locally.

It would be great to document the process a little bit in the github contributing document.

@hydrosquall
Copy link
Member Author

hydrosquall commented Jul 17, 2021

I am okay with canary releases with every pull request but I don't want releases where I have to promise backwards compatibility with every merged pull request. I want to manually decide when we make a release. Bonus points if I don't have to do anything locally.

What you describe makes sense. I propose that we follow the 2 branch scheme (see https://intuit.github.io/auto/docs/generated/shipit#next-branch-default for a picture + 4 sentence description )

I did a proof of concept of what I suggest using a small test project (hydrosquall/storybook-addon-datadog-rum#9 (comment))

Here's my proposal. If it sounds good to you to try, I'll write up a doc and add it to this PR (but feel free to ask for further questions or clarification).

Reasoning

Problem: It's valuable to automatically release the result of every merged PR approved so maintainers don't have to manually cut releases just because a 1st time contributor opened a change. However, preserving stability within semantic ranges on the latest tag is important, because not every PR is going to warrant bumping the version on the "latest" channel.

Solution:

  • Release everything on the next tag for those who want to live on the cutting-edge
  • Manually open a PR from next to main when you're ready to open a stable release

Method

To get the best of all worlds, I suggest we

  1. Create a next branch as our pre-release branch. All PRs should be made into this branch.
  2. Create a stable branch as the branch which is used for stable deploys. It's suggested that branch protection gets added to the pre-release branch to prevent direct commits, but it's not required.
  3. Make next the repository default branch so that all new PRs are opened into it
  4. Whenever you want to manually cut a release (nothing local needs to be done), open a PR from next into stable with whatever title you want. The version bump will be calculated by conventional-commits (see docs ). The default behavior is to do nothing - for example, this PR won't increment anything for the library because the prefix is build.

When all open PRs made against it are closed, I also suggest we remove the master branch so people don't try to use it.

Alternatives

  • Maintain just 1 branch (master, though I suggest renaming it if we take this route), create a latest release by (see docs) applying a github label to any PR to mark it as promotable if merged. The downside:

While this setup may be simpler, it restricts you from updating latest while development is happening for the pre-release. With 2 branches you can easily merge update to the latest release, with 1 this is not possible.

@hydrosquall
Copy link
Member Author

hydrosquall commented Jul 17, 2021

Per the vega-dev slack thread, I tried to add the magic-zero plugin, but it doesn't work. Added a comment with some debugging output for the library authors: intuit/auto#1946 (comment) , and reverted the change.

@domoritz
Copy link
Member

Thank you for explaining the process I think I like the two branches approach.

For apps like the editor, we could also have two branches and auto release the stable branch (maybe call it deployed).

Can you automatically disable canary releases for dependably updates? I think that's getting too noisy otherwise.

Paging @kanitw and @arvind to see whether they agree and want us to adopt this approach in Vega-Lite, embed, themes, tooltips etc as well.

@hydrosquall hydrosquall force-pushed the cameron.yick/release-setup-auto branch 2 times, most recently from ef0f1ba to be6e0f0 Compare July 19, 2021 22:41
@hydrosquall hydrosquall force-pushed the cameron.yick/release-setup-auto branch from be6e0f0 to f2acdcf Compare July 19, 2021 22:49
@hydrosquall
Copy link
Member Author

hydrosquall commented Jul 19, 2021

Can you automatically disable canary releases for dependably updates? I think that's getting too noisy otherwise.

Good call, updated in 9e0ff6e. This will disable the canary process on all of dependabot's branches (note it will still deploy a new pre-release version still on merge.)

@hydrosquall hydrosquall force-pushed the cameron.yick/release-setup-auto branch from 65c4b76 to 388c5d8 Compare July 20, 2021 00:18
@hydrosquall hydrosquall changed the base branch from master to next July 20, 2021 00:43
@hydrosquall
Copy link
Member Author

hydrosquall commented Jul 20, 2021

Updated base branch of this PR from master to next. After this PR is merged, we should edit the repo default branch from main to next so that new PRs will be made relative to it instead.

auto.config.ts Outdated
// Follow the 2 branch deployment scheme
// https://intuit.github.io/auto/docs/generated/shipit#next-branch-default
baseBranch: "stable", // latest "official" release
prereleaseBranches: ["main"] // latest changes (subject to breaking). next, alpha, beta, and other multi-feature test branches can all be added here.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't edited this config yet, but plan to make switch this to next after confirming it makes sense w/ @domoritz to rotate this to the default branch

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rotating the default branch makes sense to me.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to next: 492820b

I don't have admin permissions on this repo, so I'll need your help to switch the default branch to next. I just synced next to the HEAD of master (8439cfbb45918c11ac5df65e9a516a07763478a9) so we should be ready to go.

Copy link
Member

@domoritz domoritz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Let's do it (please squash the commits in here when merging).

The next steps are updating the following repos with the same proess

  • vega-lite-dev-config
  • vega-embed
  • vega-tooltip
  • vega-themes
  • vega-datasets
  • vega-lite

And then Vega if @jheer wants it.

jobs:
publish:
runs-on: ubuntu-latest
if: "!contains(github.event.head_commit.message, 'ci skip') && !contains(github.event.head_commit.message, 'skip ci')"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we use this right now? If not, let' remove it.

Copy link
Member Author

@hydrosquall hydrosquall Jul 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is necessary because auto create commits which contain ci skip) when it updates the changelog and bumps the version in the stable branch. This guard is present to prevent auto from triggering an infinite loop (creating a commit, rerunning auto, etc). (example )

Image 2021-07-20 at 9 14 22 PM

https://intuit.github.io/auto/docs/build-platforms/github-actions

plugins: [
"npm",
"conventional-commits",
"first-time-contributor",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this do?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When someone contributes to a repository for the first time, it tags and thanks them in the first Changelog heading and Release Notes , and doesn't do anything if this release doesn't have any 1st time contributors.

Docs: https://intuit.github.io/auto/docs/generated/first-time-contributor

We can remove this if it wouldn't be relevant for the Vega projects, but I figured it couldn't hurt + might help with encouraging people to become ongoing contributors.

@hydrosquall
Copy link
Member Author

hydrosquall commented Jul 21, 2021

Looks good to me. Let's do it (please squash the commits in here when merging).

Will do! By the way, within the repository settings, we can actually require squash commits for repositories if we want to prevent accidental misclicks: https://docs.github.com/en/github/administering-a-repository/configuring-pull-request-merges/configuring-commit-squashing-for-pull-requests

The next steps are updating the following repos with the same process

Sounds good, I made a card to track the migrations. I'll move these over gradually. I remember you pointed out that some of these repos may have more elaborate build processes than others

https://github.com/orgs/vega/projects/8?add_cards_query=is%3Aopen

@domoritz
Copy link
Member

Vega-Lite will be the trickiest. Have a look at the current scripts and let me know about any questions you have.

@domoritz
Copy link
Member

By the way, within the repository settings, we can actually require squash commits for repositories if we want to prevent accidental misclicks

I found that dependabot updates often have fewer conflicts when I rebase. Maybe that's not true anymore but that's why I left support for rebase on.

@hydrosquall hydrosquall merged commit 80bba1c into next Jul 22, 2021
@hydrosquall hydrosquall deleted the cameron.yick/release-setup-auto branch July 22, 2021 01:15
domoritz pushed a commit that referenced this pull request Jul 22, 2021
* build: add dependencies for auto versioning and release notes

* chore: configure publish workflow with auto shipit

* chore: ignore .env files for when running auto locally

* build: remove semantic-release dependencies and config

* build: add dep for magic-zero auto plugin

* Revert "build: add dep for magic-zero auto plugin"

This reverts commit 5eff32b.

* build: disable builds for branches opened by dependabot

* docs: update notes on the publishing process

* build: update base branch to next

Co-authored-by: Remco Haszing <remcohaszing@gmail.com>
@vega-org-bot vega-org-bot added the released This issue/pull request has been released. label Jul 22, 2021
This was referenced Oct 5, 2021
@vega-org-bot
Copy link
Collaborator

🚀 PR was released in v0.97.0-next.0 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Update one or more dependencies version prerelease released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants