react-native-version icon indicating copy to clipboard operation
react-native-version copied to clipboard

Git commit -a is dangerous

Open jpolo opened this issue 6 years ago • 3 comments

Hi, We are using react-native-version very often and it appears it uses git commit -awhen creating the version. This leads to a quite dangerous behavior when the local repo of the person releasing the project is not clean. It has two nasty effects

  • it adds unwanted changes to git
  • it hides the changes in the version commit with the message "vX.X.X".

I've used other version script and it seems the standard would be to abort the command, if the repo is not absolutely clean, forcing the dev to stash all modifications.

jpolo avatar Mar 07 '19 20:03 jpolo

Hi @jpolo, could you provide an example of the workflow you do with RNV?

Just to be sure we're on the same page, RNV doesn't create a new version. A new version is created by the npm version command which you should be executing before RNV.

RNV uses git commit -a in only two situations:

  1. When it's executed from the "postversion" script - which is triggered by npm version. Make sure you're not running this script manually (npm run postversion). You can however, add the --never-amend option and commit the changes yourself.

  2. When it's executed with the --amend option. In this case, there is a clear intent that you want to amend the previous commit. But again, it's expected that you just executed npm version, which already guards your Git working directory the way you describe.

stovmascript avatar Mar 09 '19 09:03 stovmascript

Yes we use in postversion hook with no args. I had some uncommitted unadded changes and I ran yarn version. In the mean time I saw that we also use a custom changelog hook I'll check this one. Nevertheless don't you think the git commit all is dangerous without a git status check before?

jpolo avatar Mar 09 '19 20:03 jpolo

I see the problem now. Unlike npm version, which checks and aborts if the Git working directory is dirty, yarn version doesn't care and just makes a version commit. If you have uncommited changes to package.json, it will result in the same situation which you described. That's pretty messy of Yarn if you ask me.

RNV is relying on checks made by npm version.

Nevertheless, I guess we're gonna have to improve support for Yarn and make our own checks. In the meanwhile, using npm version should be safe, if you don't mind the missing "v" in its commit message.

stovmascript avatar Mar 09 '19 21:03 stovmascript