Last modified: 2012-03-26 23:36:08 UTC

Wikimedia Bugzilla is closed!

Wikimedia has migrated from Bugzilla to Phabricator. Bug reports should be created and updated in Wikimedia Phabricator instead. Please create an account in Phabricator and add your Bugzilla email address to it.
Wikimedia Bugzilla is read-only. If you try to edit or create any bug report in Bugzilla you will be shown an intentional error message.
In order to access the Phabricator task corresponding to a Bugzilla report, just remove "static-" from its URL.
You could still run searches in Bugzilla or access your list of votes but bug reports will obviously not be up-to-date in Bugzilla.
Bug 35456 - git-review for branches
git-review for branches
Status: RESOLVED WORKSFORME
Product: Wikimedia
Classification: Unclassified
Git/Gerrit (Other open bugs)
unspecified
All All
: Unprioritized normal (vote)
: ---
Assigned To: Nobody - You can work on this!
:
Depends on:
Blocks: 22596
  Show dependency treegraph
 
Reported: 2012-03-24 17:07 UTC by Sam Reed (reedy)
Modified: 2012-03-26 23:36 UTC (History)
7 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Sam Reed (reedy) 2012-03-24 17:07:29 UTC
Currently, git review cannot be used on branches.

Which means only people with core review access can commit directly to those branches.

As ialex brought up, there is no way to request the merges to the branches, other than asking a core review to cherry pick and push.


So this brings 2 points; We have a bunch of core committers who probably should be allowed to commit to these branches. Maybe some of these should also have review privileges for core too, but that's a different discussion.

Also, we need a way to people to request pulls into branches. Having git-review work would be nice, as would pressing a button in gerrit. Going back to having to add requests to wikipages etc is going in the wrong direction.
Comment 1 Marcin Cieślak 2012-03-24 17:15:46 UTC
The magic part is "refs/for/REL1_19" insted of "refs/for/master"

Here's what I did:

$ git checkout gerrit/REL1_19
(...)
$ git branch REL1_19
$ echo .... >> RELEASE-NOTES-1.19
$ git add RELEASE-NOTES-1.19
$ git commit -m "Testing for bug 35456"
[detached HEAD f4b682a] Testing for bug 35456
 1 files changed, 1 insertions(+), 0 deletions(-)
$ git push gerrit HEAD:refs/for/REL1_19
Counting objects: 5, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (3/3), done.
Writing objects: 100% (3/3), 371 bytes, done.
Total 3 (delta 2), reused 0 (delta 0)
remote: Resolving deltas:   0% (0/2)
remote: 
remote: New Changes:
remote:   https://gerrit.wikimedia.org/r/3688
remote: 
To ssh://saper@review:29418/mediawiki/core.git
 * [new branch]      HEAD -> refs/for/REL1_19

So we have Gerrit change #3688
Comment 2 Sam Reed (reedy) 2012-03-24 17:34:11 UTC
<gerrit-wm> New patchset: saper; "Testing for bug 35456" [mediawiki/core] (REL1_19) - https://gerrit.wikimedia.org/r/3688
<Reedy> lol wut
<saper> git-review is too magic to me
<Reedy> git-review wouldn't work for me when I tried it earlier this week...
<saper> it wasn't git-review, that's why it worked
<Reedy> Did you use git push origin REL1_1X ?
<saper> Reedy: refs/for/REL1_19
<Reedy> Hmmm
<Reedy> saper: guess it should be at least documented :p
<saper> Reedy: it is documented very well
<saper> in gerrit docs
<Reedy> Lol
<saper> but we give workarounds
Comment 3 Marcin Cieślak 2012-03-26 10:26:07 UTC
It seems that you can use -R to tell git-review not to rebase against .gitreview's idea of what the current branch is.

Added link to the upstream bug (https://bugs.launchpad.net/mediawiki/+bug/883176)
Comment 4 Antoine "hashar" Musso (WMF) 2012-03-26 15:59:47 UTC
git-review accepts as an optional argument the branch name to interact with. When that argument is not specified, it fallback to look for the defaultbranch parameter in the .gitreview file.

Every branch should have a .gitreview having a correct defaultbranch value. We probably do not want to force people to use something like:  `git-review REL1_19`

Output example using test/mediawiki/core and a .gitreview file NOT having default branch set:

$ git-review -v REL1_19
2012-03-26 17:57:30.318755 Running: git log --color=never --oneline HEAD^1..HEAD
2012-03-26 17:57:30.335452 Running: git remote
2012-03-26 17:57:30.340266 Running: git branch -a --color=never
2012-03-26 17:57:30.346184 Running: git branch --color=never
2012-03-26 17:57:30.351537 Running: git log HEAD^1..HEAD
Found topic 'REL1_19' from parsing changes.
2012-03-26 17:57:30.372507 Running: git rev-parse --show-toplevel
2012-03-26 17:57:30.377682 Running: git remote update gerrit
Fetching Gerrit change #2012-03-26 17:57:32.224581 Running: git rebase -i remotes/gerrit/REL1_19
2012-03-26 17:57:32.505708 Running: git config --get color.ui
2012-03-26 17:57:32.510419 Running: git log --color=always --decorate --oneline REL1_19 --not remotes/gerrit/REL1_19
2012-03-26 17:57:32.531614 Running: git push gerrit HEAD:refs/for/REL1_19/REL1_19
remote: Resolving deltas:   0% (0/2)
remote: 
remote: New Changes:
remote:   https://gerrit.wikimedia.org/r/3742
remote: 
To ssh://hashar@gerrit.wikimedia.org:29418/test/mediawiki/core.git
 * [new branch]      HEAD -> refs/for/REL1_19/REL1_19
$


Now with a defaultbranch specified:


$ git-review -v
2012-03-26 17:59:01.654284 Running: git log --color=never --oneline HEAD^1..HEAD
2012-03-26 17:59:01.671722 Running: git remote
2012-03-26 17:59:01.678109 Running: git branch -a --color=never
2012-03-26 17:59:01.684509 Running: git branch --color=never
2012-03-26 17:59:01.690706 Running: git log HEAD^1..HEAD
Found topic 'REL1_19' from parsing changes.
2012-03-26 17:59:01.711494 Running: git rev-parse --show-toplevel
2012-03-26 17:59:01.716346 Running: git remote update gerrit
Fetching Gerrit change #2012-03-26 17:59:03.550708 Running: git rebase -i remotes/gerrit/REL1_19
2012-03-26 17:59:03.848351 Running: git config --get color.ui
2012-03-26 17:59:03.853355 Running: git log --color=always --decorate --oneline REL1_19 --not remotes/gerrit/REL1_19
You have more than one commit that you are about to submit.
The outstanding commits are:

598114c (HEAD, REL1_19) yeah defaultbranch rocks
22570d4 yah testing file for confirmation

Is this really what you meant to do?
Type 'yes' to confirm: yes
2012-03-26 17:59:07.938479 Running: git push gerrit HEAD:refs/for/REL1_19/REL1_19
remote: Resolving deltas:   0% (0/2)
remote: 
remote: New Changes:
remote:   https://gerrit.wikimedia.org/r/3743
remote: 
To ssh://hashar@gerrit.wikimedia.org:29418/test/mediawiki/core.git
 * [new branch]      HEAD -> refs/for/REL1_19/REL1_19
$

Thus,  I am marking this issue as work for me
Comment 5 Antoine "hashar" Musso (WMF) 2012-03-26 16:04:36 UTC
I have sent .gitreview files for mediawiki/core.git with changes :

REL1_19  https://gerrit.wikimedia.org/r/3744
REL1_18  https://gerrit.wikimedia.org/r/3745
REL1_17  https://gerrit.wikimedia.org/r/3746
Comment 6 Sam Reed (reedy) 2012-03-26 16:16:55 UTC
(In reply to comment #5)
> I have sent .gitreview files for mediawiki/core.git with changes :
> 
> REL1_19  https://gerrit.wikimedia.org/r/3744
> REL1_18  https://gerrit.wikimedia.org/r/3745
> REL1_17  https://gerrit.wikimedia.org/r/3746

Great thanks!

It did seem that just getting some clarification and updating the documentation would've been enough to suffice dealing with this bug :)
Comment 7 Antoine "hashar" Musso (WMF) 2012-03-26 18:38:36 UTC
Basic doc updated at : https://www.mediawiki.org/wiki/Git/Workflow#acting_on_remote_branches
Comment 8 Marcin Cieślak 2012-03-26 23:36:08 UTC
I think we should work towards git-review without the need of .gitreview file at all. There is a work underway (https://review.openstack.org/#change,5720) to not need .gitreview at all to determine repository access and derive it fully from existing git remotes as well as using any remote (https://review.openstack.org/#change,5784) instead of hardcoded "gerrit".

I think it is also possible to get branch name out of gerrit metadata (at least when subsequent changes are submitted, but for the first change it might probably be derived from HEAD).

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links