unexpected results when canceling RC deploy

[pixelzoom]

I started doing an RC deploy. When I answered "N" to "Is it ready to deploy?", it continued to do something (checking out master?), and then failed with the Error shown below.

First, it would be nice if it told you that it was reverting to master, not continuing to deploy.

Second, I'm wondering if this Error is even really an error. It looks like something that's expected.

Assigning to @ariel-phet to prioritize and assign.

% cd diffusion
% grunt rc --brands=phet --branch=1.0
...
Please test the built version of diffusion.
Is it ready to deploy [y/N]?N
info: Setting version from package.json for diffusion to 1.0.0-rc.0
info: git add package.json on diffusion
info: git commit on diffusion with message:
Bumping version to 1.0.0-rc.0
info: git push on diffusion to 1.0
info: checking out master for diffusion
info: getting dependencies.json for diffusion
info: git checkout master on assert
info: git checkout master on axon
info: git checkout master on brand
info: git checkout master on chipper
info: git checkout master on diffusion
info: git checkout master on dot
info: git checkout master on gas-properties
info: git checkout master on joist
info: git checkout master on kite
info: git checkout master on phet-core
info: git checkout master on phet-io
info: git checkout master on phet-io-wrappers
info: git checkout master on phetcommon
info: git checkout master on phetmarks
info: git checkout master on query-string-machine
info: git checkout master on scenery
info: git checkout master on scenery-phet
info: git checkout master on sherpa
info: git checkout master on sun
info: git checkout master on tambo
info: git checkout master on tandem
info: git checkout master on twixt
info: npm update on diffusion
info: npm update on chipper
>> Detected failure during deploy, reverting to master
info: checking out master for diffusion
info: getting dependencies.json for diffusion
info: git checkout master on assert
info: git checkout master on axon
info: git checkout master on brand
info: git checkout master on chipper
info: git checkout master on diffusion
info: git checkout master on dot
info: git checkout master on gas-properties
info: git checkout master on joist
info: git checkout master on kite
info: git checkout master on phet-core
info: git checkout master on phet-io
info: git checkout master on phet-io-wrappers
info: git checkout master on phetcommon
info: git checkout master on phetmarks
info: git checkout master on query-string-machine
info: git checkout master on scenery
info: git checkout master on scenery-phet
info: git checkout master on sherpa
info: git checkout master on sun
info: git checkout master on tambo
info: git checkout master on tandem
info: git checkout master on twixt
info: npm update on diffusion
info: npm update on chipper
Fatal error: Perennial task failed:
Error: Aborted rc deployment (aborted version change too).
  at module.exports (/Users/cmalley/PhET/GitHub/perennial/js/grunt/rc.js:111:13)
  at process._tickCallback (internal/process/next_tick.js:68:7)

Full Error details:
{}
Fatal error: perennial grunt "--repo=diffusion" "rc" "--brands=phet" "--branch=1.0" failed with code 1
%

[jonathanolson]

What would the ideal behavior be? It sounds like a message immediately after saying it's not ready would be good (instead of having the message be part of the thrown error, so that it would print "Aborted rc deployment (reverting the version change and checking out master)" near the top?

An error/throw is useful, so that it will flag things as "not successfully completed".

Relevant code:

    if ( !await booleanPrompt( `Please test the built version of ${repo}.\nIs it ready to deploy`, noninteractive ) ) {
      // Abort version update
      await setRepoVersion( repo, previousVersion, message );
      await gitPush( repo, branch );

      // Abort checkout
      await checkoutMaster( repo, true );
      throw new Error( 'Aborted rc deployment (aborted version change too).' );
    }

[pixelzoom]

Something like:

Is it ready to deploy [y/N]?N
Reverting work done for deploy.
...

"Reverting version to 1.0.0-rc.0" instead of "Bumping version to 1.0.0-rc.0" would be a bonus.

And definitely not throwing an Error. The user requested to abort the deploy, and the process is doing that correctly. If it couldn't abort, then an Error would be appropriate.