[DOCS] git-tag-version=false does not commit

[y-nk]

Is there an existing issue for this?

• I have searched the existing issues

Current Behavior

the npm version command does not perform the git commit operation.

Expected Behavior

the npm version command still performs the git commit operation.

Steps To Reproduce

1. In any repo
2. Run npm version --git-tag-version=false
3. the package.json will be modified, but the git commit won't be there.

Environment

• OS: irrelevant
• Node: latest
• npm: 7.18.1

[y-nk]

PS: it's specified in the documentation that the git-tag-version options "tags the commit", so it should be expected that turning it to false only "does not tag the commit", implicitly that the commit should still exist.

[isaacs]

Yeah, we don't commit it if we're not creating the tag. That's how it's been for at least 2 majors now, and changing that would be a breaking change.

I think this just needs a docs fix, tbh.

[y-nk]

@isaacs the parameter's name would still be misleading imho.

[isaacs]

@y-nk That's a fair complaint, but it's also been that way since "use git when doing npm version" has been around, so it's a disruptive breaking change to make the behavior different.

We could split the config into two different options (say, version-git-commit and version-git-tag or something) in npm v9 with the upcoming config refactor, but we'd likely keep the existing config option as an alias for backwards compatibility.

[isaacs]

We could split the config into two different options (say, version-git-commit and version-git-tag or something)

Actually, that'd be kind of weird, because setting version-git-tag without version-git-commit would be an error, so one would have to imply the other. The plan is to get away from cases where one config flag has to imply another, because that just creates a lot of side effects and weirdness today that's hard to reason about.

So maybe the best option would be something like a --version-git=("tag", "commit", false) enum option? Idk, just spitballing.

[wraithgar]

Made a PR to update docs so that this is clarified. Having this flag do something different than it does now should go through the rfc process