Migrate prepare-release-from-ci to new workflow #20581

acdlite · 2021-01-13T18:40:33Z

I added a --releaseChannel (-r) argument to the script. You must choose either "stable" or "experimental", because every build job now includes both channels.

The prepare-release-from-npm script is unchanged since those releases are downloaded from npm, not CI.

(As a side note, I think we should start preparing semver releases using the prepare-release-from-ci script, too, and get rid of prepare-release-from-npm. I think downloading from npm was a neat idea originally, but because we already run npm pack before storing the artifacts in CI, there's really not much additional safety; the only safeguard it adds is the requirement that a "next" release must have already been published.)

Test plan

Prepare a stable release:

./scripts/release/prepare-release-from-ci.js --skipTests -r stable

Prepare an experimental release:

./scripts/release/prepare-release-from-ci.js --skipTests -r experimental

acdlite · 2021-01-13T18:44:25Z

scripts/release/shared-commands/download-build-artifacts.js

-  const compressedPackages = readdirSync(join(cwd, 'build/node_modules/'));
-  for (let i = 0; i < compressedPackages.length; i++) {
-    await exec(
-      `tar zxvf ./build/node_modules/${compressedPackages[i]} -C ./build/node_modules/`,


When this was originally added, we didn't store the entire build directory as a CI artifact. Just the npm packages.

The reason we stored each module as an individual tarball was because we ran npm pack to run any npm pre-processing, like filtering files out based on the package.files array. The build script already does this, though, so we can skip the extra step.

sizebot · 2021-01-13T18:46:17Z

No significant bundle size changes to report.

Size changes (experimental)

Generated by 🚫 dangerJS against 6ea9ed6

sizebot · 2021-01-13T18:46:22Z

No significant bundle size changes to report.

Size changes (stable)

Generated by 🚫 dangerJS against 6ea9ed6

codesandbox-ci · 2021-01-13T18:50:29Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 6ea9ed6:

Sandbox	Source
React	Configuration

bvaughn · 2021-01-14T13:50:24Z

(As a side note, I think we should start preparing semver releases using the prepare-release-from-ci script, too, and get rid of prepare-release-from-npm. I think downloading from npm was a neat idea originally, but because we already run npm pack before storing the artifacts in CI, there's really not much additional safety; the only safeguard it adds is the requirement that a "next" release must have already been published.)

What's the main motivation for this change? Seems like that's a nice safeguard, though it's admittedly not totally necessary. What additional benefit do we get from publishing from CI?

bvaughn

👍

bvaughn · 2021-01-14T13:51:20Z

scripts/release/prepare-release-from-ci.js

+    const channel = params.releaseChannel;
+    if (channel !== 'experimental' && channel !== 'stable') {
+      console.error(
+        `Invalid release channel: "${channel}". Must be "stable" or "experimental".`


May help clarify a tad to call out the param name too?

Suggested change

`Invalid release channel: "${channel}". Must be "stable" or "experimental".`

`Invalid release channel (-r) "${channel}". Must be "stable" or "experimental".`

Might be better still to follow precedent here and add this validation into parse-params.js e.g.

react/scripts/release/publish-commands/parse-params.js

Lines 35 to 46 in a656ace

params.tags.forEach(tag => {

switch (tag) {

case 'latest':

case 'next':

case 'experimental':

case 'untagged':

break;

default:

console.error('Unknown tag: "' + params.tag + '"');

process.exit(1);

break;

}

acdlite · 2021-01-14T16:13:10Z

What's the main motivation for [getting rid of the prepare from npm script]? Seems like that's a nice safeguard, though it's admittedly not totally necessary. What additional benefit do we get from publishing from CI?

Not much other than fewer things to maintain, or accidentally subtly break.

I added a `--releaseChannel (-r)` argument to script. You must choose either "stable" or "experimental", because every build job now includes both channels. The prepare-release-from-npm script is unchanged since those releases are downloaded from npm, nt CI. (As a side note, I think we should start preparing semver releases using the prepare-release-from-ci script, too, and get rid of prepare-release-from-npm. I think that was a neat idea originally but because we already run `npm pack` before storing the artifacts in CI, there's really not much additional safety; the only safeguard it adds is the requirement that a "next" release must have already been published.)

rickhanlonii · 2021-01-15T12:17:48Z

Is the goal for this to eventually support www-modern and www-classic too?

hecerinc · 2021-03-15T00:50:38Z

scripts/release/shared-commands/download-build-artifacts.js

+  // Download and extract artifact
+  await exec(`rm -rf ./build2`, {cwd});
+  await exec(
+    `curl -L $(fwdproxy-config curl) ${buildArtifacts.url} | tar -xvz`,


Hey @acdlite I'm trying to execute this script on a Windows machine and it looks like it assumes Unix heavily. I'm trying to rewrite this in a more xplat fashion, but I can't figure out what fwdproxy-config curl is or returns. If you could point me to the package that enables this command or something, that'd be great. Thanks!

@acdlite friendly ping

* Migrate prepare-release-from-ci to new workflow I added a `--releaseChannel (-r)` argument to script. You must choose either "stable" or "experimental", because every build job now includes both channels. The prepare-release-from-npm script is unchanged since those releases are downloaded from npm, nt CI. (As a side note, I think we should start preparing semver releases using the prepare-release-from-ci script, too, and get rid of prepare-release-from-npm. I think that was a neat idea originally but because we already run `npm pack` before storing the artifacts in CI, there's really not much additional safety; the only safeguard it adds is the requirement that a "next" release must have already been published.) * Move validation to parse-params module

acdlite requested review from bvaughn and rickhanlonii January 13, 2021 18:40

facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Jan 13, 2021

acdlite commented Jan 13, 2021

View reviewed changes

bvaughn approved these changes Jan 14, 2021

View reviewed changes

acdlite added 2 commits January 14, 2021 11:06

Move validation to parse-params module

6ea9ed6

acdlite force-pushed the migrate-release-script branch from 3a0bf84 to 6ea9ed6 Compare January 14, 2021 17:12

acdlite merged commit 98313aa into facebook:master Jan 14, 2021

hecerinc reviewed Mar 15, 2021

View reviewed changes

hecerinc mentioned this pull request Mar 15, 2021

Update devtools-extensions build script to reflect changes in local b… #21004

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate prepare-release-from-ci to new workflow #20581

Migrate prepare-release-from-ci to new workflow #20581

acdlite commented Jan 13, 2021 •

edited

acdlite Jan 13, 2021

sizebot commented Jan 13, 2021 •

edited

sizebot commented Jan 13, 2021 •

edited

codesandbox-ci bot commented Jan 13, 2021 •

edited

bvaughn commented Jan 14, 2021

bvaughn left a comment

bvaughn Jan 14, 2021

bvaughn Jan 14, 2021

acdlite commented Jan 14, 2021 •

edited

rickhanlonii commented Jan 15, 2021

hecerinc Mar 15, 2021

hecerinc Mar 18, 2021

	`Invalid release channel: "${channel}". Must be "stable" or "experimental".`
	`Invalid release channel (-r) "${channel}". Must be "stable" or "experimental".`

	params.tags.forEach(tag => {
	switch (tag) {
	case 'latest':
	case 'next':
	case 'experimental':
	case 'untagged':
	break;
	default:
	console.error('Unknown tag: "' + params.tag + '"');
	process.exit(1);
	break;
	}

Migrate prepare-release-from-ci to new workflow #20581

Migrate prepare-release-from-ci to new workflow #20581

Conversation

acdlite commented Jan 13, 2021 • edited

Test plan

acdlite Jan 13, 2021

Choose a reason for hiding this comment

sizebot commented Jan 13, 2021 • edited

Size changes (experimental)

sizebot commented Jan 13, 2021 • edited

Size changes (stable)

codesandbox-ci bot commented Jan 13, 2021 • edited

bvaughn commented Jan 14, 2021

bvaughn left a comment

Choose a reason for hiding this comment

bvaughn Jan 14, 2021

Choose a reason for hiding this comment

bvaughn Jan 14, 2021

Choose a reason for hiding this comment

acdlite commented Jan 14, 2021 • edited

rickhanlonii commented Jan 15, 2021

hecerinc Mar 15, 2021

Choose a reason for hiding this comment

hecerinc Mar 18, 2021

Choose a reason for hiding this comment

acdlite commented Jan 13, 2021 •

edited

sizebot commented Jan 13, 2021 •

edited

sizebot commented Jan 13, 2021 •

edited

codesandbox-ci bot commented Jan 13, 2021 •

edited

acdlite commented Jan 14, 2021 •

edited