-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
docs(releasing): add schedule, more explicit integration testing #9695
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice docs! 📖
|
||
# Run the tests again :) | ||
# Get that new changelog commit + one last test. | ||
bash ./lighthouse-core/scripts/release/test.sh | ||
# Package everything for publishing | ||
bash ./lighthouse-core/scripts/release/prepare-package.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mention making a release in GH to get the x.x.x tag, which is required for prepare-package
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's not a required step for making a tag, the prepare-package
script makes the tag. but I do need to add the whole "add GH Release" thing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fwiw, this should also be a very easy automatable step given a GITHUB_KEY in your env variables.
Co-Authored-By: Shane Exterkamp <shaneexterkamp5@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as previously discussed, I think this is more documentation of our release tech debt than a release process, but it's good to have it in writing as a starting point for improving :)
@@ -4,7 +4,23 @@ | |||
|
|||
### Cadence | |||
|
|||
We ship once a month, on the Thursday before the 1st. While not necessary, followup minor/patch releases may be done if warranted. The planned ship dates are added to the internal Lighthouse calendar. | |||
We aim to release every 3 weeks. Our schedule is set as follows: One day before the [expected Chromium branch point](https://www.chromium.org/developers/calendar) (which is every six weeks) and again exactly 3 weeks after that day. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't feel comfortable writing this down with no established history of being able to pull this off and without a bot doing the releasing, but I guess I'm comfortable with you taking the heat for it, @connorjclark :)
git pull | ||
git new-branch lh-roll-x.x.x | ||
gclient sync | ||
autoninja -C out/Release chrome blink_tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how bad is this without goma?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh it's unbearable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
woah I've never even heard of autoninja 😮
# See: https://www.chromium.org/developers/how-tos/get-the-code | ||
|
||
# Roll to Chromium folder. | ||
yarn devtools |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@paulirish wasn't there a way (used to be a way?) to live-edit devtools? Could we do that with Lighthouse so that the releaser isn't dependent on rebuilding?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yahh there is a thing now. --custom-devtools-frontend
https://chromium-review.googlesource.com/c/chromium/src/+/1791150
this might just work for us. but might not because remote modules.
# Note: if the changes include proto changes make sure that the API has those new fields. | ||
# If anything is wrong, stop releasing, investigate, and prioritize landing the PR. | ||
|
||
# For bonus points, add some tests covering new features. Either a new test, or an extra |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as for devtools
docs/releasing.md
Outdated
```sh | ||
# Run the internal `update_lighthouse_assets.sh` script. | ||
|
||
# Update that silly string that sets the version number for metrics. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it seems less useful to have these lines here than just to say to test it (the internal doc has step by step directions)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed... this line is actually uncovered in the internal docs, but I've updated them now.
# https://g3doc.corp.google.com/chrome/headless/lightrider/README.md?cl=head#test-lr-changes-in-canary | ||
|
||
# Verify that Lightrider works properly, and is generating reports fully. Consider the new features that have been added. | ||
# Note: if the changes include proto changes make sure that the API has those new fields. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we automate this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe @exterkamp would be interested in doing this in the future
docs/releasing.md
Outdated
|
||
# Run the tests again :) | ||
# Get that new changelog commit + one last test. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure what this is referring to. From an earlier draft?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated .. the test.sh does two important things here (tests, and updates to latest code)
docs/releasing.md
Outdated
|
||
# Update that silly string that sets the version number for metrics. | ||
|
||
# Test things out locally, if happy, deploy to canary and see how the graphs react. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how long will this typically take?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated. not long to see if something went really bad, which is all Canary is good for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still have lots of concerns about the release process, but modulo remaining comments, docs changes LGTM :)
|
||
# Run the tests again :) | ||
# One last test (this script uses origin/master, so we also get the commit with the new changelog - that commit should be HEAD). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this step be run in pristine?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The release scripts cd to pristine. You always run them from a development checkout.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I stumbled onto this PR again from link in chat and discovered these pending comments. 1 or 2 still seemed useful to have for the record, but 8 months too late or whatever 😆
@@ -25,35 +43,104 @@ Release manager follows the below _Release Process_. | |||
|
|||
### Versioning | |||
|
|||
We follow [semver](https://semver.org/) versioning semantics (`vMajor.Minor.Patch`), to align with the greater Node community. Generally, this means our bi-weekly releases will bump a minor. Though we will release a new patch version if high-priority fixes are required before the next schedule release. Additionally, if a schedule release contains no new features, then we'll only bump the patch version. | |||
We follow [semver](https://semver.org/) versioning semantics (`vMajor.Minor.Patch`). Breaking changes will bump the major version. New features or bug fixes will bump the minor version. If a release contains no new features, then we'll only bump the patch version. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
worth clarifying what constitutes a "breaking change" here?
probably enough meat on that question for a separate PR but wanted to leave the thought in here all the same :)
|
||
# Run the tests again :) | ||
# Get that new changelog commit + one last test. | ||
bash ./lighthouse-core/scripts/release/test.sh | ||
# Package everything for publishing | ||
bash ./lighthouse-core/scripts/release/prepare-package.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fwiw, this should also be a very easy automatable step given a GITHUB_KEY in your env variables.
bash ./lighthouse-core/scripts/release/test.sh | ||
# Package everything for publishing | ||
bash ./lighthouse-core/scripts/release/prepare-package.sh | ||
|
||
# Make sure you're in the Lighthouse pristine repo we just tested. | ||
cd ../lighthouse-pristine | ||
|
||
# Publish to NPM | ||
# Sanity check: last chance to abort. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should/can we add flags to these to get them to fail if somethings not right? i.e. we're looking for a porcelain git status and the master/tagged version commit in git log
?
git pull | ||
git new-branch lh-roll-x.x.x | ||
gclient sync | ||
autoninja -C out/Release chrome blink_tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
woah I've never even heard of autoninja 😮
|
||
### The following Monday | ||
|
||
Evaluate LR staging, if all looks good, promote to production! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any link here back to the comms around LR?
https://github.com/GoogleChrome/lighthouse/blob/releasing-docs/docs/releasing.md