fix: Warn when defaulting to --inc=patch in CLI by pjohnmeyer · Pull Request #859 · npm/node-semver (original) (raw)

Adds a warning log message to the CLI when the --increment option is provided with an invalid increment type. This provides adequate feedback to the user about what is happening, and prepares them for a future where this may no longer be allowed.

In #108 (comment), @mklement0 points out (correctly, IMO):

Quietly accepting invalid input is problematic.

As @lukekarrys points out in his reply, however, he considers erroring here to be a breaking change. I would tend to agree as this behavior has become part of the contract between the CLI user and the CLI.

The issue is being tracked for the next major version of the CLI, so I offer up that we can safely add the warning indicating that a bad value has been provided, simultaneously preserving the contract, and preparing users for the possibility of a future change. As console.warn does not output to stdout, existing redirection and/or piping of output should survive this change, thus it will not be breaking. Examples:

% ./bin/semver.js --inc patchouli 3.2.1 > ./new-version.redirected
Invalid value for --inc; defaulting to 'patch'. This may become a failure in future major versions.
% cat ./new-version.redirected
3.2.2

% ./bin/semver.js --inc patchouli 3.2.1 | tee ./new-version.teed
Invalid value for --inc; defaulting to 'patch'. This may become a failure in future major versions.
3.2.2
% cat ./new-version.teed 
3.2.2

The only breakage, then, would occur if we consider stderr (or in this case lack thereof) output to also be part of the CLI contract. Example:

% ./bin/semver.js --inc patchouli 3.2.1 &> ./new-version.err    
% cat ./new-version.err 
Invalid value for --inc; defaulting to 'patch'. This may become a failure in future major versions.
3.2.2

In this case the warning and the new version are both output to ./new-version.err, and any processing of that output expecting only 3.2.2 might fail.

Alternative

We could, instead, add a flag to the CLI to opt-in to new behavior re: the --inc flag. This could be done in three different ways I can immediately think of:

--increment-strict: an alternate flag used instead of --increment that enforces the validity of the argument; downside is that we would then want to remove this in the next release, creating another breaking change
--future: a generic flag that hides all breaking "next version" (currently 8) functionality behind a feature flag, used in combination with --increment to enforce the validity of the argument
--verbose: adds the additional "warning" logging added in this PR, with the opportunity for more usage elsewhere

I think the approach in this PR is the simplest and requires the least change in the future, but --future and --verbose open up other options going forward.

References

Relates to #108