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

Updated release flow #118 #141

Merged
merged 10 commits into from
May 18, 2020
Merged

Updated release flow #118 #141

merged 10 commits into from
May 18, 2020

Conversation

suzubara
Copy link
Contributor

@suzubara suzubara commented May 5, 2020

New & improved release workflow 🎉

I've made these changes with the desire to better streamline the release flow process, which is currently 100% manual. That said, this PR does not fully automate releases -- I want to make sure we're being very deliberate with every step, and not try to configure full end-to-end automation pre-maturely. I believe these changes will introduce minimal disruption into our current process, but make enough change to put us on track towards automated releases.

  • Added commitlint, commitizen, and standard-version to enforce standard commits & release notes
  • Updates CHANGELOG to use conventional commit format
  • Remove CHANGELOG requirement from dangerfile (changelog updates will be auto-generated at the time of release based on commit messages now)
  • Updated Contributing doc to reflect the changes in dev/release flow
  • Run stylelint as part of CI linting
  • Move build command to prepublishOnly (so the project doesn't get built on every yarn install)
  • Ran prettier on all files

There are 2 outstanding things I would like to continue working on, but felt the changes on this branch were far enough along that it can be reviewed and merged as-is. The big win in this PR is getting commit linting into the project ASAP so that our next set of release notes can be generated automatically. New issues for the remaining TODOs are:

Fixes #118

yarn audit v1.13.0
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ low           │ Prototype Pollution                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ yargs-parser                                                 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=13.1.2 <14.0.0 || >=15.0.1 <16.0.0 || >=18.1.2             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @storybook/storybook-deployer                                │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ @storybook/storybook-deployer > yargs > yargs-parser         │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/1500                      │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ low           │ Prototype Pollution                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ yargs-parser                                                 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=13.1.2 <14.0.0 || >=15.0.1 <16.0.0 || >=18.1.2             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ standard-version                                             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ standard-version > yargs > yargs-parser                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/1500                      │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ low           │ Prototype Pollution                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ yargs-parser                                                 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=13.1.2 <14.0.0 || >=15.0.1 <16.0.0 || >=18.1.2             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @commitlint/cli                                              │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ @commitlint/cli > @commitlint/lint > @commitlint/parse >     │
│               │ conventional-commits-parser > meow > yargs-parser            │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/1500                      │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ low           │ Prototype Pollution                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ yargs-parser                                                 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=13.1.2 <14.0.0 || >=15.0.1 <16.0.0 || >=18.1.2             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ standard-version                                             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ standard-version > conventional-recommended-bump >           │
│               │ conventional-commits-parser > meow > yargs-parser            │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/1500                      │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ low           │ Prototype Pollution                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ yargs-parser                                                 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=13.1.2 <14.0.0 || >=15.0.1 <16.0.0 || >=18.1.2             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ standard-version                                             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ standard-version > conventional-changelog >                  │
│               │ conventional-changelog-core > conventional-commits-parser >  │
│               │ meow > yargs-parser                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/1500                      │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ low           │ Prototype Pollution                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ yargs-parser                                                 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=13.1.2 <14.0.0 || >=15.0.1 <16.0.0 || >=18.1.2             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ standard-version                                             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ standard-version > conventional-changelog >                  │
│               │ conventional-changelog-core > conventional-changelog-writer  │
│               │ > meow > yargs-parser                                        │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/1500                      │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ low           │ Prototype Pollution                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ yargs-parser                                                 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=13.1.2 <14.0.0 || >=15.0.1 <16.0.0 || >=18.1.2             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ standard-version                                             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ standard-version > conventional-changelog >                  │
│               │ conventional-changelog-core > git-semver-tags > meow >       │
│               │ yargs-parser                                                 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/1500                      │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ low           │ Prototype Pollution                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ yargs-parser                                                 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=13.1.2 <14.0.0 || >=15.0.1 <16.0.0 || >=18.1.2             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @commitlint/cli                                              │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ @commitlint/cli > @commitlint/read > git-raw-commits > meow  │
│               │ > yargs-parser                                               │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/1500                      │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ moderate      │ Regular Expression Denial of Service                         │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ marked                                                       │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=0.6.2                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @storybook/addon-info                                        │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ @storybook/addon-info > marksy > marked                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/812                       │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ low           │ Prototype Pollution                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ yargs-parser                                                 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=13.1.2 <14.0.0 || >=15.0.1 <16.0.0 || >=18.1.2             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @commitlint/cli                                              │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ @commitlint/cli > meow > yargs-parser                        │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/1500                      │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ low           │ Prototype Pollution                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ yargs-parser                                                 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=13.1.2 <14.0.0 || >=15.0.1 <16.0.0 || >=18.1.2             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ standard-version                                             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ standard-version > conventional-recommended-bump >           │
│               │ git-semver-tags > meow > yargs-parser                        │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/1500                      │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ low           │ Prototype Pollution                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ yargs-parser                                                 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=13.1.2 <14.0.0 || >=15.0.1 <16.0.0 || >=18.1.2             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ standard-version                                             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ standard-version > git-semver-tags > meow > yargs-parser     │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/1500                      │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ low           │ Prototype Pollution                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ yargs-parser                                                 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=13.1.2 <14.0.0 || >=15.0.1 <16.0.0 || >=18.1.2             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ standard-version                                             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ standard-version > conventional-recommended-bump > meow >    │
│               │ yargs-parser                                                 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/1500                      │
└───────────────┴──────────────────────────────────────────────────────────────┘
13 vulnerabilities found - Packages audited: 348334
Severity: 12 Low | 1 Moderate
✨  Done in 2.08s.

For example: `sr-accordion-component-112`

### Conventional Commits

Copy link
Contributor

@haworku haworku May 5, 2020

Choose a reason for hiding this comment

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

I think we should either describe or link here to examples of how commit messages should look for this standard.

The most important part (in my mind) is that people are aware of how to highlight a breaking changes specifically in their message. Particularly since we may use that to eventually automate the version bump down the road.

CHANGELOG.md Outdated
@@ -1,38 +1,29 @@
# Changelog

All notable changes to this project will be documented in this file.
All notable changes to this project will be documented in this file. See [standard-version](https://github.com/conventional-changelog/standard-version) for commit guidelines.
Copy link
Contributor

Choose a reason for hiding this comment

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

When is the changelog to be updated now? Is there still a manual change needed when merging new work? i.e. the developer should update changelog with unreleased changes before merging into develop? Or are we only updating the changelog now on version releases with standard-version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The latter - changelog will only be updated on version releases using standard-version

This project uses a few tools to help standardize contributions and a streamlined release flow:

- [`commitlint`](https://github.com/conventional-changelog/commitlint) git hook, using the [conventional commits config](https://github.com/conventional-changelog/commitlint/tree/master/%40commitlint/config-conventional)
- [`commitizen`](https://commitizen.github.io/cz-cli/) CLI for making it easy to write standard commit messages
Copy link
Contributor

Choose a reason for hiding this comment

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

should we also reference standard-version again here?


2. Create the release candidate branch (note that version numbers should _never_ be prefixed with `v`):
### Releasing
Copy link
Contributor

@sojeri sojeri May 6, 2020

Choose a reason for hiding this comment

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

I thought the point of this change (esp using semantic release) was to make every merge to develop trigger a release based on the merge commit message.

These new docs sound like we still want to follow a manual release process...?

npm publish
```
```
npm publish
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we doing this from dev machines rather than from CI?

@suzubara
Copy link
Contributor Author

@sojeri The point is to have a more automated change, but I don't want to run before we can walk, and felt that using semantic release right off the bat on this project was overkill. This update will establish a standard for commit messages so we can start having an auto-generated changelog & version numbers, which will take most of the currently manual work out of the release flow. Determining how and when to do fully automated new stable releases is later down the line IMO.

- Prettier (auto-formatting) and eslint are run on _staged files_ as a pre-commit hook
- For an optimal developer experience, it's recommended that you configure your editor to run linting/formatting inline.
- All tests must pass, and eslint is run on all files in CircleCI
(TODO) Passing `develop` builds will automatically be published to the `next` tag on NPM. This allows users to easily test out the latest version in their applications, which may be unstable.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Calling this out as a TODO - need to add GH workflow for this

```

Publishing access is limited to package owners. If you need access and don't have it, please contact `@npm-admins` on Slack.
1. Once the release PR is approved, complete the release and publish the new version (this should be automated by GH - TODO):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Calling this out as a TODO - need to add GH workflow for this

@trussworks-infra-zz
Copy link

trussworks-infra-zz commented May 11, 2020

Warnings
⚠️ This PR does not include changes to storybook, even though it affects component code.
⚠️ This PR does not include changes to tests, even though it affects source code.

Generated by 🚫 dangerJS against b7b8462

@suzubara suzubara marked this pull request as ready for review May 11, 2020 20:42
@suzubara suzubara requested a review from ahobson May 11, 2020 20:46
chalk "2.4.2"
get-stdin "7.0.0"
lodash "4.17.15"
meow "5.0.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

This meow dependency is bringing in more of the bad yargs-parser versions. I'm seeing at least one commitlint/cli PR attempting to address this, so it may get patched soon.

standard-version suffers the same issue, though it's more indirect (through conventional-recommended-bump and mocha). This one doesn't have an open issue or PR yet, so may take longer to get patched if we don't engage.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This meow dependency is bringing in more of the bad yargs-parser versions. I'm seeing at least one commitlint/cli PR attempting to address this, so it may get patched soon.

standard-version suffers the same issue, though it's more indirect (through conventional-recommended-bump and mocha). This one doesn't have an open issue or PR yet, so may take longer to get patched if we don't engage.

this was mentioned in our check-in today, but for posterity: we're going to add an ADR around our stance re: merging in dependencies that have open vulnerabilities, but tldr is that they should not block work, especially if low severity, and especially if in devDependencies (which won't end up in production code)

@suzubara suzubara requested review from sojeri and haworku May 13, 2020 21:24
@suzubara suzubara merged commit 0c995cd into develop May 18, 2020
@suzubara suzubara deleted the sr-automated-release-118 branch May 18, 2020 17:30
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 this pull request may close these issues.

Determine standards & conventions for streamlining release process
4 participants