Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [mdt-ocl.dev] Playing with gerrit

Hi Ed, Laurent

Comments in-lined below.

On 18/07/2013 12:53, Ed Willink wrote:
Hi

The earlier discussion was leaving me a bit discouraged, but..
On 18/07/2013 12:23, Laurent Goubet wrote:
For big features, we still develop on branches, but we try never to
push them on the eclipse repository. Not only are branches dangerous
(you have to remember never to rebase a branch that has been pushed to
a remote repository), but reviewing a long stream of commits seems
useless : what we wish to review is the end result (tip of the dev
branch), and how it changes from the existing state (master at the
time of review, not at the branching time). Reviewing such branches is
tedious and error-prone. Plus, IMHO, merging makes for a very messy
and illegible history.
I very much agree. If you look at the OCL history, I don't think you
will find a non-fast forward merge that I have done in the last year. I
always rebase onto master if I can, but too often cherry pick one by
one. The many branches on OCL are actually far more controlled than you
might think. In the long term there is just the master and maintenance
branches, with some old side branches for attempted improvemnts that
have yet to be completed/archived. In the short term there are sometimes
multiple improvments underway but they get linearised before I push to
master.
I think that Gerrit may be used with our current work methodology (branching policy), that is, rather than sharing branches to review, we share code reviews.

The point which I'm not clear enough right now is, if our methodology can be improved/simplified by using Gerrit.


When we think that the new feature is ready, we prepare it for review
by squashing the commits into a single one with a proper commit
message, which we push for review. What you see in the review I've
linked here (https://git.eclipse.org/r/#/c/13903/) is the result of
such a squash.
For a single change I certainly prefer to compare the start/finish; I
don't have time to study the mistakes that were made along the way.

Yes, comparing the first and last commit, should give you this view (avoiding intermediate corrected bugs). The question here is if we want to lose intermediate commits. It's true that in the not-useful commits which we want to distinguish we also need to include mistaken commits. In the squash step, the comment of the corresponding mistaken commit would be included though. I've not exercised amended commits, may be this could be helpful to bloat the history with mistaken commits (no idea about this integration in EGit), I'm only know the reverse commit, which basically "ammends" commit creating the inverse commit (don't like).

Unfortunately Adolfo's second try at a review request failed to squash
correctly, so I'm a bit worried that an intelligent user is making such
a mess of a trivial change.

I did it correctly from the point of view of the (second) gerrit change. The point is that second gerrit change depends on (fixes) the first change. So, to submit the second gerrit change would have implied to submit the first disapproved one. This doesn't make sense. In some how this is like a chain of mistaken-fixing commits we could currently have, though.

In a normal case (now that I know how the change-id works), this second change review should not have ever occur, but a second patchset of the firs gerrit code review.

I'm not impressed by the web interface to

Me neither. I like synchronization view from Eclipse.

Gerrit and apparently there is no working EGIT integration,

No, actually squashing sounds tedious. Basically if you have to squash 10 commits, need console (git bash):
1. Execute a command.
2. Have a textual editor (vi) where you have 10 entries, from which you select (by textually replacing "pick" by "squash") which commits are squashed.

Maybe, a 10 commits could be just be 5 if we exercise the  ammend practice.

so I had to
change to Eclipse, navigate to the file to verify that the Gerrit
display was wrong.

I didn't understood this.

It's 'just' a matter of better discipline to try to avoid two
improvements getting tangled, which is very difficult unless you have a
very fast review response, and that is counterproductive because often I
like an improvement to have a little time to settle since it was often
triggered by another problem, and may deserve further work.

In my opinion, I see two benefits:
- A probable more agile way to provide fixes to a specific problem. You don't have to create new branches, and bloat the repository with new stuff (to further delete)
- Automatic hudson verification.

I'm not sure if the drawbacks we are envisioning counter the benefits. The only point I scare is having to squash a lot of commits.

In any case, it looks like a very good solution for external contributors.

Cheers,
Adolfo.

     Regards

         Ed Willink

_______________________________________________
mdt-ocl.dev mailing list
mdt-ocl.dev@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/mdt-ocl.dev


Back to the top