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: add github action for release #4659

Merged
merged 6 commits into from
Apr 13, 2022

Conversation

lukekarrys
Copy link
Contributor

@lukekarrys lukekarrys commented Mar 31, 2022

This adds an additional GitHub action that only runs when the head branch name starts with release/. This guarantees that test-all will run even now that we publish from latest so the release PRs only contain a few files.

@lukekarrys lukekarrys requested a review from a team as a code owner March 31, 2022 23:52
@lukekarrys lukekarrys force-pushed the release/this-is-named-release-so-actions-will-run branch from 16151fb to e2fa064 Compare April 1, 2022 00:11
@npm-robot
Copy link
Contributor

npm-robot commented Apr 1, 2022

no statistically significant performance changes detected

timing results
app-large clean lock-only cache-only cache-only
peer-deps
modules-only no-lock no-cache no-modules no-clean no-clean
audit
npm@8 67.527 ±1.89 32.440 ±0.85 22.159 ±0.91 25.115 ±1.51 3.880 ±0.09 3.705 ±0.09 3.087 ±0.05 15.078 ±0.04 2.941 ±0.01 4.128 ±0.03
#4659 60.463 ±7.62 35.358 ±0.03 21.970 ±0.15 24.710 ±0.43 3.836 ±0.13 3.759 ±0.10 3.106 ±0.07 15.157 ±0.26 2.952 ±0.04 4.351 ±0.03
app-medium clean lock-only cache-only cache-only
peer-deps
modules-only no-lock no-cache no-modules no-clean no-clean
audit
npm@8 41.070 ±1.04 27.502 ±0.33 16.237 ±0.09 17.338 ±0.30 3.422 ±0.13 3.357 ±0.02 3.138 ±0.09 11.107 ±0.20 2.803 ±0.03 3.885 ±0.00
#4659 42.344 ±1.06 27.190 ±0.06 16.627 ±0.32 17.866 ±0.15 3.370 ±0.01 3.393 ±0.03 3.159 ±0.06 11.091 ±0.04 2.907 ±0.05 3.903 ±0.05

@lukekarrys lukekarrys marked this pull request as draft April 4, 2022 17:55
@lukekarrys
Copy link
Contributor Author

Converting to draft while we wait for test-all to be runnable again.

@lukekarrys lukekarrys force-pushed the release/this-is-named-release-so-actions-will-run branch 6 times, most recently from f64563d to b2cff2c Compare April 6, 2022 23:54
@lukekarrys lukekarrys marked this pull request as ready for review April 7, 2022 00:10
@lukekarrys lukekarrys force-pushed the release/this-is-named-release-so-actions-will-run branch 18 times, most recently from c48f145 to 2094ad3 Compare April 7, 2022 06:50
smoke-tests/index.js Outdated Show resolved Hide resolved
@lukekarrys lukekarrys marked this pull request as draft April 7, 2022 20:31
@lukekarrys lukekarrys force-pushed the release/this-is-named-release-so-actions-will-run branch from 41e8395 to fd77ade Compare April 8, 2022 15:28
@lukekarrys lukekarrys marked this pull request as ready for review April 8, 2022 15:28
@lukekarrys lukekarrys force-pushed the release/this-is-named-release-so-actions-will-run branch from fd77ade to c5aa3ba Compare April 8, 2022 15:30
@lukekarrys lukekarrys force-pushed the release/this-is-named-release-so-actions-will-run branch from c5aa3ba to 6a1d4ce Compare April 8, 2022 15:49

publish: gitclean ls-ok link test smoke-tests docs prune
@git push origin :v$(shell node bin/npm-cli.js --no-timing -v) 2>&1 || true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Our release process always includes us creating the tag before running this step, so this was unnecessary. It wasn't erroring but would show the message that the tag already existed.

- os: macos-latest
shell: bash
# XXX: this should be possible in windows also
# but resetdeps cant be a bash script
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 is almost ready to run in Windows, but needs resetdeps and the rm -rf to be cross platform. I'm saving that for a future PR.

@lukekarrys
Copy link
Contributor Author

Refactored this based on input from @nlf. Notable changes:

  • When running smoke-publish we install a packed tarball of the current repo from a temp dir, delete the contents of the dir, and run the smoke tests with only npm to ensure we are testing the newly installed version
  • Versions the global install with a prerelease id of $GITHUB_SHA and then tests that npm -v returns that
  • smoke-tests is a workspace meaning that npm run test-all will also run the smoke tests
  • Removed the matrix of smoke-tests from the default CI action. It still runs in ubuntu + node 16 to catch common mistakes but the full matrix is only run on release. I think this is a good tradeoff between speed of our everyday CI and testing everything before release.

@lukekarrys lukekarrys requested a review from a team April 11, 2022 18:07
@fritzy fritzy merged commit ac9c2a6 into latest Apr 13, 2022
@fritzy fritzy deleted the release/this-is-named-release-so-actions-will-run branch April 13, 2022 21:20
@lukekarrys lukekarrys mentioned this pull request Apr 14, 2022
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.

4 participants