Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Wikipedia: Wikitech

consecutive commits in Gerrit

 

 

Wikipedia wikitech RSS feed   Index | Next | Previous | View Threaded


amir.aharoni at mail

Mar 25, 2012, 5:23 AM

Post #1 of 5 (1094 views)
Permalink
consecutive commits in Gerrit

Hi,

There's something in Gerrit that i still don't get, even after trying
to ask this several times on mediawiki.org [1] and on IRC. I sincerely
thank everyone who took the time to reply, but either i still
misunderstand the replies or i do understand them and I Just Don't
Like It [2].

Let's consider a patch set i submitted:
https://gerrit.wikimedia.org/r/#change,3361 .

The patch set as i submitted it was not perfect and Aaron marked it
"-1". So far, so good. Then Hashar amended the patch set and
Nikerabbit amended it some more. Now here's what i don't like: The
diffs of their changes show all the changes instead of just showing
Hashar's and Nikerabbit's changes. This is somewhat similar to how we
review patches by new volunteer developers - if the patch attached to
a Bugzilla is not perfect we ask to write a whole new one. That's the
custom, and maybe it's considered educational, but actually its
usefulness is doubtful. Despite this, it is now applied to all the
commits.

Now honestly, the result in the story of this particular patch set is
OK. The resulting patch is better than the first one and the
collaboration worked well. But does this scale? This patch set changes
only a few lines of code; what will happen with patch sets in which
dozens of lines are changed? These happen very often. The reviewer
will have to re-read all the changed lines for every amend and this
looks like a huge waste of time.

It would make a lot more sense to me to see these three changes as a
branch with three commits - the first one is mine, the second one is
Hashar's fix and the third one is Nikerabbit's fix. When there's
agreement that the tip of the branch is good, it's merged to the
master branch. If a reviewer wants to see the whole end-to-end diff,
it's not hard. Mind you, this has nothing to do with SVN - in SVN
branches are used rarely. This, to the best of my understanding, is
the Git way. Git makes this sensible scenario easy, and it is used
successfully in Github - but in the Gerrit workflow this scenario is
not used. I tried reading the Gerrit manual [3] and it didn't make it
any clearer. Yet again, i may be misunderstanding something, so
correct me if i'm wrong; It's really weird that a tool that makes code
review so central makes it a lot harder, too. But that's the feeling
that i get.

Am i just wrong in my understanding of Gerrit?
Am i the only one who doesn't like it?
Is it too late to complain?

[1] https://www.mediawiki.org/wiki/Talk:Git/Workflow
[2] As in https://en.wikipedia.org/wiki/Wikipedia:IDONTLIKEIT .
[3] http://gerrit-documentation.googlecode.com/svn/Documentation/2.2.2/index.html

--
Amir Elisha Aharoni · אָמִיר אֱלִישָׁע אַהֲרוֹנִי
http://aharoni.wordpress.com
‪“We're living in pieces,
I want to live in peace.” – T. Moore‬

_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


amir.aharoni at mail

Mar 25, 2012, 5:48 AM

Post #2 of 5 (1040 views)
Permalink
Re: consecutive commits in Gerrit [In reply to]

2012/3/25 Amir E. Aharoni <amir.aharoni [at] mail>:
> Hi,
>
> There's something in Gerrit that i still don't get, even after trying
> to ask this several times on mediawiki.org [1] and on IRC. I sincerely
> thank everyone who took the time to reply, but either i still
> misunderstand the replies or i do understand them and I Just Don't
> Like It [2].
>
> Let's consider a patch set i submitted:
> https://gerrit.wikimedia.org/r/#change,3361 .
>
> The patch set as i submitted it was not perfect and Aaron marked it
> "-1". So far, so good. Then Hashar amended the patch set and
> Nikerabbit amended it some more.

... Now that i look at that patch again, i realize that there were no
code changes by Nikerabbit - only inline code comments. That confusion
was probably caused by Gerrit's UI, with which many people already
expressed dissatisfaction, but that's a separate issue.

My main claim still applies - it would make more sense to see every
stage in the development of this patch as a separate commit. I tried
to play with the "Old Version History" dropdown, which looked like it
can help making it easier, but it only added more confusion.

--
Amir

_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


Platonides at gmail

Mar 25, 2012, 7:00 AM

Post #3 of 5 (1032 views)
Permalink
Re: consecutive commits in Gerrit [In reply to]

There is a dropdown called "Old Version History" from which you can diff
from a previous patchset instead of viewing the whole patch (yes, I had
to be told about it).
It's not perfect though, and for instance the commit message is always
compared as if it were new.

You can also compare it by downloading with git review, but the only way
I found requires you to copy the sha1 ids, which makes a horrible usability.

I like your proposal of doing patchsets as a branch. Sometimes the new
patchset are better viewed as a completely different work, in which case
it could be based from the parent, but most subsequent patchsets fit
better in the branch model.
Branched patchsets could then be moved to master as a merge (which is
not only better from a history POV, but also in tracking the authorship)
or as a cherry-pick (eg. for a typo fix)


_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


amir.aharoni at mail

Mar 25, 2012, 1:20 PM

Post #4 of 5 (1034 views)
Permalink
Re: consecutive commits in Gerrit [In reply to]

2012/3/25 Platonides <Platonides [at] gmail>:
> There is a dropdown called "Old Version History" from which you can diff
> from a previous patchset instead of viewing the whole patch (yes, I had
> to be told about it).

I actually saw it myself and i even mentioned it.

If i try to use it, the file
"includes/filerepo/backend/FileBackendStore.php" appears in the list
of modified files, even though it has nothing to do with any of these
commits. It has "+0, -54" in the "size" column, which is supposed to
mean that 54 lines were removed, and when i actually click it, then
the diff is empty.

Is that supposed to happen?

--
Amir Elisha Aharoni · אָמִיר אֱלִישָׁע אַהֲרוֹנִי
http://aharoni.wordpress.com
‪“We're living in pieces,
I want to live in peace.” – T. Moore‬

_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


Platonides at gmail

Mar 25, 2012, 3:43 PM

Post #5 of 5 (1043 views)
Permalink
Re: consecutive commits in Gerrit [In reply to]

On 25/03/12 22:20, Amir E. Aharoni wrote:
> 2012/3/25 Platonides <Platonides [at] gmail>:
>> There is a dropdown called "Old Version History" from which you can diff
>> from a previous patchset instead of viewing the whole patch (yes, I had
>> to be told about it).
>
> I actually saw it myself and i even mentioned it.
>
> If i try to use it, the file
> "includes/filerepo/backend/FileBackendStore.php" appears in the list
> of modified files, even though it has nothing to do with any of these
> commits. It has "+0, -54" in the "size" column, which is supposed to
> mean that 54 lines were removed, and when i actually click it, then
> the diff is empty.

Yes, it seems that is still refers to the parent revision, even though
it is now compared to another changeset. Although if you go to the diff,
it shows empty for example (comparing to the other changeset).


> Is that supposed to happen?

I find that a confusing interface. I have no idea if it's really
intended or not.


_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Wikipedia wikitech RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.