misc: more releasing tweaks #9772
Conversation
| @@ -60,7 +60,7 @@ and that no new PRs should be merged until you are done. | |||
| ```sh | |||
| # Run the tests. | |||
| bash ./lighthouse-core/scripts/release/test.sh | |||
| # Change into the pristine folder. | |||
| # Change into the newly-created pristine folder. | |||
There was a problem hiding this comment.
this is confusing unless you realize it was just created for you
| @@ -114,10 +114,6 @@ Confirm Lightrider integration will work: | |||
| Now that the integrations are confirmed to work, go back to `lighthouse` folder. | |||
|
|
|||
| ```sh | |||
| # Run the tests again. | |||
| # Note: you can skip this if you didn't need to land any changes. | |||
| bash ./lighthouse-core/scripts/release/test.sh | |||
There was a problem hiding this comment.
this doesn't really make sense here (if you've just had to land a PR you should be starting over)
| @@ -23,9 +23,6 @@ yarn build-all | |||
| echo "Running the standard test suite..." | |||
| yarn test | |||
|
|
|||
| echo "Running the smoke tests...." | |||
| yarn smoke || yarn smoke || yarn smoke | |||
There was a problem hiding this comment.
this is not really necessary.
- in theory, master is green.
yarn testis a nice sanity check, but running all the smoke tests is overkill because - they're about to be run again in the packaged pristine version and
- it's incredibly annoying to have to run this all over again if master is still green but there's some hiccup further down in this script :)
| @@ -39,7 +36,7 @@ cd /tmp/lighthouse-local-test | |||
|
|
|||
| npm init -y | |||
| npm install "$LH_PRISTINE_ROOT/lighthouse-$VERSION.tgz" | |||
| npm explore lighthouse -- npm run smoke | |||
| npm explore lighthouse -- npm run smoke || npm explore lighthouse -- npm run smoke | |||
There was a problem hiding this comment.
and it's nice to run this twice just in case there's some flake (like I just experienced)
There was a problem hiding this comment.
should we make it three like we just removed above? :)
There was a problem hiding this comment.
wanna lift the bash-ism i used in #9766 ?
There was a problem hiding this comment.
FWIW connor that actually required significant cognitive overhead for me to verify it's doing what we want it to do. when it's only a couple times, I'd greatly prefer || <repeat>
There was a problem hiding this comment.
all seems good to me!
over to @connorjclark since I believe some of these were your specific additions based on your experiences?
| @@ -39,7 +36,7 @@ cd /tmp/lighthouse-local-test | |||
|
|
|||
| npm init -y | |||
| npm install "$LH_PRISTINE_ROOT/lighthouse-$VERSION.tgz" | |||
| npm explore lighthouse -- npm run smoke | |||
| npm explore lighthouse -- npm run smoke || npm explore lighthouse -- npm run smoke | |||
There was a problem hiding this comment.
should we make it three like we just removed above? :)
lgtm |
|
Can you add somewhere to mark the expected Stable chrome version in the release notes? |
already there |
another iteration of trying to streamline this thing :)