"Git bump commit" should have been reverted if the the command "git tag" fails

[marcellodesales]

Hi there! Your plugin is awesome! I maintain multiple modules and I manage versions manually every day... Yours is definitely helping me a lot!!!

So, as I'm adopting it, some tags are higher than the version of the package.json.... (those days I was super sleepy to verify the manual process :zzz:)... The version on the package.json was old by 1 so it collided...

mdesales@ubuntu [11/15/201422:30:33] ~/dev/github-intuit/sp-quality (master *+) $ gulp patch
[22:30:41] Using gulpfile ~/dev/github-intuit/sp-quality/Gulpfile.js
[22:30:41] Starting 'patch'...
[22:30:41] Bumped version to: 0.3.3
[22:30:41] [master d30d31c] bumps package version
 1 file changed, 1 insertion(+), 1 deletion(-)

[22:30:41] Tagging as: 0.3.3
[22:30:41] Finished 'patch' after 62 ms
[22:30:41] { [Error: Command failed: fatal: tag '0.3.3' already exists
] killed: false, code: 128, signal: null }
[22:30:41]  fatal: tag '0.3.3' already exists

Wit that, I was expecting the plugin to now keep the "bad" commit there... But it did...

^Cmdesales@ubuntu [11/15/201422:30:51] ~/dev/github-intuit/sp-quality (master *+) $ git lol
* d30d31c (HEAD, master) bumps package version
* d299f24 ISP-354: Refactoring the main module file to follow the Checkstyle :D

thanks!

[marcellodesales]

I tried solving the problem but I can't... I was trying to use the convention from the Gulp documentation, but I can't handle the error... https://github.com/stevelacy/gulp-git#gittagversion-message-opt-cb

    git.tag(tag, 'tagging as '+tag, opts, function (err) {
      if (err) {
        gutil.log('Reverting bump commit...');
        git.reset('HEAD^');
      }
    });

That did not work... The handler does not work... :(

[marcellodesales]

I think I have a proposed solution to use the git module "gift" to retrieve the list of tags and verify if the "candidate" one already exists.

[ikari-pl]

Hi! I understand your problem, and agree that an attempt to re-create already existing tag may cause a lot of trouble. But I'd like to simplify the problem rather than the hack-around a bit:

You should admit that in the perfect world, the situation of having mismatch between tag and package.json should never happen, especially that it breaks npm big time ;-) I don't live in a perfect world, and have panickly removed a premature tag from the remote too, so I'm not innocent.

The plugin should follow the Unix philosophy of "do one thing, do it well". Maybe I posted a too wide example (that I use myself) that includes version bumping scenarios, but please remember, that neither version bumping, nor commiting the bump is part of this plugin's functionality. Those are just the tasks that I assumed we always do before tagging a commit with a version.

Because of the last, we can't really revert the last commit --- the plugin has no idea what was in the last commit (it didn't do it). We can't remove commits blindly!

What we should do, I believe, is to gracefully emit an error (in other words, most likely, throw an exception) inside the plugin, since an unexpected condition was encountered and it is to the gulpscript maintainer to handle this situation (we shouldn't make things even more unexpected).

I think the check whether the tag already exist is a great idea and I'll look into gift in a free moment — maybe it would be best to entirely replace gulp-git dependency with gift. It smells to keep two dependencies that do the same thing (even with different APIs).

And that means making sure that the options that were passed down to gulp-git would still work (see https://github.com/ikari-pl/gulp-tag-version/pull/2). On the other hand, this might be a perfect moment to break backwards compatibility with https://github.com/ikari-pl/gulp-tag-version/pull/2 a little, and move the options to a separate namespace, just to make it clear, which parts go to this plugin, and which go to git. I made a mistake by mixing options for two plugins/commands in one object and on the same level.

So, I now have something to work on in this repository, just give me a while, I usually check it here once in a couple of days :) I will probably soon use your commit and create a new version taking all above into consideration.

[marcellodesales]

Hi!

Yes, thanks for your valuable insights... I do agree with you when it comes to solving the problem right... However, although I believe that it's better to give something that does not break, I see the point of the graceful error messages...

Yeah, I would suggest using gift... I really liked experimenting it because it gave me full control over the git functionalities I was looking for...

Thanks for the great plugin anyway! It's been very useful in my daily work managing private plugins where NPM repository is still controlled using git repos.

[ikari-pl]

Hi, So I included your patch, switched to gift exclusively and tried to just report an error in case of failure. Fortunately it seems that if i just throw in my module, it will be converted to an emitted error by the stream map module that I use. Does the current version under https://github.com/ikari-pl/gulp-tag-version/tree/check_duplicate_tag look sane to you? :) Since I replaced the git module completely, I believe it's a breaking change (so, major version bump). It might break usages like suggested by issue #2 .

[marcellodesales]

@ikari-pl The change seem to be sane :D Considering it is 2:50am and I spent the weekend working with Go :P

I will definitely pull and test it out tomorrow, but reading the code I did not see anything alarming... Great idea on checking with the author of #2 ... I tried to reading his requirements, but not sure what the translation of the command would look like.. If he replies to you, I can certainly help debugging it if needed...

Thanks again!

[ikari-pl]

Well, in theory, since I want to stick to proper semver versioning, I am going to double check that I bumped the major, which means "backwards-compatibility breaking changes happened" and, in theory, he probably would have to upgrade manually to the newer version anyway ;-) (auto-update shouldn't break his package, no matter how the option[s] were used…) Nice, seems we're separated by 9 hours time zone difference.

[ikari-pl]

You know, I believe a decent sleep schedule would fix half od your problems ;-)

[marcellodesales]

@ikari-pl Yeah... I hope he won't need to change the version... :+1:

Hah my brain does not let me :zzz: sometimes :) Yeah perfect time zone difference for online work!