[Wireshark-dev] Is git-review safe?

2014-03-13 Thread Hadriel Kaplan
So a funny thing happened while using 'git review' tonight.

I was in a local branch named aruba_erm_radio, did my changes, did git 
commit, and then did git review.

Inside my commit message, in the second paragraph, I mentioned that it resolves 
an enhancement bug 9880.

For some reason, git-review decided to therefore name the branch on gerrit 
bug/9880, instead of aruba_erm_radio.

That seems a bit dangerous to me.  What happens if my initial commit+review 
doesn't have a bug identified in the commit message, but in a later update I 
add one into the amended commit-message?  Would it try to push it to a 
different branch on gerrit?  Or would the Change-ID being the same prevent that?

-hadriel

p.s. the evidence:

Hadriel-MacBook-2:wireshark hadrielk$ git status
On branch aruba_erm_radio
Changes not staged for commit:
  (use git add file... to update what will be committed)
  (use git checkout -- file... to discard changes in working directory)

modified:   epan/dissectors/packet-aruba-erm.c


It took 2.57 seconds to enumerate untracked files. 'status -uno'
may speed it up, but you have to be careful not to forget to add
new files yourself (see 'git help status').
no changes added to commit (use git add and/or git commit -a)
Hadriel-MacBook-2:wireshark hadrielk$ git commit -a
[aruba_erm_radio e6ffa92] Add support for Aruba ERM Radio-Format
 1 file changed, 97 insertions(+), 10 deletions(-)
Hadriel-MacBook-2:wireshark hadrielk$ git review
remote: Resolving deltas: 100% (4/4)
remote: Processing changes: new: 1, refs: 1, done
remote:
remote: New Changes:
remote:   https://code.wireshark.org/review/629
remote:
To ssh://hadr...@code.wireshark.org:29418/wireshark
 * [new branch]  HEAD - refs/publish/master/bug/9880

___
Sent via:Wireshark-dev mailing list wireshark-dev@wireshark.org
Archives:http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe


Re: [Wireshark-dev] Is git-review safe?

2014-03-13 Thread Evan Huus
Apparently it tries to be smart and guess the appropriate topic. The relevant 
function is [1].

As far as safe goes, I think it is. This is only the Gerrit topic, not target 
branch, so the worst that happens is that pushing a new changeset will actually 
change the topic on you. You can always force a topic with the -t flag if you 
really don't want that to happen for some reason.

Git review has their own bug tracker, mailing list, etc. if you feel like 
tracking this down further, but I'm not too concerned at the moment. Good catch 
though.

Evan

[1] 
https://github.com/openstack-infra/git-review/blob/master/git_review/cmd.py#L581

 On Mar 13, 2014, at 3:03 AM, Hadriel Kaplan hadriel.kap...@oracle.com wrote:
 
 So a funny thing happened while using 'git review' tonight.
 
 I was in a local branch named aruba_erm_radio, did my changes, did git 
 commit, and then did git review.
 
 Inside my commit message, in the second paragraph, I mentioned that it 
 resolves an enhancement bug 9880.
 
 For some reason, git-review decided to therefore name the branch on gerrit 
 bug/9880, instead of aruba_erm_radio.
 
 That seems a bit dangerous to me.  What happens if my initial commit+review 
 doesn't have a bug identified in the commit message, but in a later update I 
 add one into the amended commit-message?  Would it try to push it to a 
 different branch on gerrit?  Or would the Change-ID being the same prevent 
 that?
 
 -hadriel
 
 p.s. the evidence:
 
 Hadriel-MacBook-2:wireshark hadrielk$ git status
 On branch aruba_erm_radio
 Changes not staged for commit:
  (use git add file... to update what will be committed)
  (use git checkout -- file... to discard changes in working directory)
 
modified:   epan/dissectors/packet-aruba-erm.c
 
 
 It took 2.57 seconds to enumerate untracked files. 'status -uno'
 may speed it up, but you have to be careful not to forget to add
 new files yourself (see 'git help status').
 no changes added to commit (use git add and/or git commit -a)
 Hadriel-MacBook-2:wireshark hadrielk$ git commit -a
 [aruba_erm_radio e6ffa92] Add support for Aruba ERM Radio-Format
 1 file changed, 97 insertions(+), 10 deletions(-)
 Hadriel-MacBook-2:wireshark hadrielk$ git review
 remote: Resolving deltas: 100% (4/4)
 remote: Processing changes: new: 1, refs: 1, done
 remote:
 remote: New Changes:
 remote:   https://code.wireshark.org/review/629
 remote:
 To ssh://hadr...@code.wireshark.org:29418/wireshark
 * [new branch]  HEAD - refs/publish/master/bug/9880
 
 ___
 Sent via:Wireshark-dev mailing list wireshark-dev@wireshark.org
 Archives:http://www.wireshark.org/lists/wireshark-dev
 Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
___
Sent via:Wireshark-dev mailing list wireshark-dev@wireshark.org
Archives:http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe