search

mydrive / capistrano-deploytags

Add timestamped Git tags for each environment on deployment
BSD 2-Clause "Simplified" License
122 stars 20 forks source link

correctly handle failing git fetch #17

Closed aspiers closed 10 years ago

aspiers commented 10 years ago

If the repository doesn't have an upstream branch configured, git fetch will output an error; handle this separately to prevent it interfering with the git diff call which checks to see if we have any pending changes.

relistan commented 10 years ago

Great idea, the PR is much appreciated. I'd implement it as a two-liner using the safe_run method we already use elsewhere for this.

safe_run('git fetch')
!(`git diff #{branch} --shortstat`.strip.empty?)

If you don't mind updating the PR, I'll merge it!

aspiers commented 10 years ago

Hmm, that won't work quite as well, for two reasons. Firstly, safe_run raises a normal exception resulting in an ugly stacktrace. Secondly, it doesn't capture the output from git fetch so the message git fetch failed! will appear after it, and consequently won't show an explicit connection between the two. (In my code, the colon in the error message git fetch failed: makes it very obvious that the following error is from git.) The second point is less significant, but the combination of the two does makes it harder to see what the real error was, and I'm a big believer in anything which makes the development process less taxing on the brain ;-)

aspiers commented 10 years ago

Any more thoughts on this? Thanks!

relistan commented 10 years ago

I'll take a look at this over the weekend. Been swamped here, sorry for the delay!

aspiers commented 10 years ago

No problem and thanks a lot in advance!

relistan commented 10 years ago

Merged this, but it broke a bunch of tests so I reworked the code and test suite to pass all the tests. I also made it raise a stack trace as I actually prefer that so that I can tell where things went wrong. I think this is a good addition, thanks for the contribution!

Released on rubygems.org as 0.9.0. It would be much appreciated if you can test that this still works for your use case.