|
Continuing with in-lined comments.
On 18/07/2013 12:26, Adolfo Sanchez-Barbudo Herrera wrote:
On
18/07/2013 10:42, Laurent Goubet wrote:
Adolfo,
To be honest I don't like this. Merging a potential long bunch
of
commits, in which everyone has its purpose and comments, in
just one,
makes me lose one of the essence of this agile quick changes
oriented
methodology which I love.
I am not a fan of that requirement from gerrit myself, but one
could
argue that it makes for a much more legible history, since you
have your
features properly isolated in their commits.
You do not lose the history
of who did what, theirs comments and why they changed a
particular
review : that history is logged on gerrit instead, through the
comments
and various patch sets. For example, on
https://git.eclipse.org/r/#/c/13903/, you can see a "big"
feature-introducing commit. Gerrit keeps an history of the four
distinct
patch sets that have been created for that particular feature
along with
the comments we made during the reviews (on each patch set).
I prefer, features isolated by branching/merging. In one commit,
you can't distinguish useful changes from those not so useful
(regenarated code, releng changes). Moreover, if several people
have been participating on a feature I don't know how authoring
will be attributed in a squashed commit. I guess that in this
situations, there will be different gerrit code review
(dependent?). Anyway, in principle having to squash commits to
request code review doesn't sound good to me.
Merging this with the next answer.
One would just need to look at the OCL
repository's history to feel that
gerrit is an improvement :p. I really have trouble finding out
what
commit introduced what change for which purpose in the (too)
many
branches in there. Gerrit's "one commit for a change" tends to
make for
a much clearer policy.
I'd have to think how our current workflow (branches policy for
example) would change with gerrit. Actually, I'd like to see what
happens when a change is successfully reviewed and submitted. My
current gerrit experiment is currently based on master's commits.
Many concerns:
- Are you, replacing a branch per feature/bug policy with this
Gerrit usage ?
- What happens if the are several live pending code reviews?.
- Are they automatically merged when approved in case other
approved reviews arrived before?
- What about of integrity/integration of the merged independent
reviews (one could break the other)? What about if there are
conflicts?
To me it looks like we still need a branches based development. In
that case, rather than a clearer history one commit per
feature/bug only ensures a shorter one ^^.
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.
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.
The workflow described on
https://wiki.openstack.org/wiki/Gerrit_Workflow is very similar to
our use of git and gerrit, but there might be other, more sensible
workflows (maybe something to ask on the EGit mailing list?).
I'll try this squeezed commit, perhaps
it's the cause of the push
rejection. I guess that EGit doesn't have a way to this via
UI, does it ?
That, I do not know. I rarely squash commits into one. Amending
is
usually much easier when you work on a change :).
Some feelings I've got so far. It also
requires be more careful when
committing/pushing, which could be error prone. Now we have
push and
push to gerrit, I'm used to "commit and push" button (a normal
push).
It sounds dangerous.
True enough, even very recently I've pushed to the repository a
few
changes for which the review had not been completed. But that
can be
avoided through a number of means :
* disable the force push on your repository (you can no longer
bypass
gerrit)
o On Compare, we've kept the force-push ability in order
to avoid
gerrit reviews when pushing the stream of very small
changes I
usually need when fixing build errors
Yes, having to pass through gerrit doesn't make sense. It would
have to be a useless workflow of "I send a code review request",
and "I approve my self".
Which you can actually do, however useless :p.
* Make it so that your "normal" remote's
push configuration is for
gerrit. If your look at
http://wiki.eclipse.org/EMF_Compare/Gerrit#Adding_a_new_remote,
we
add a "review" remote to our repositories so that we can
push to
gerrit via "git push review". This configuration can also be
made to
be your default instead of a new remote, so that "git push"
pushes
on gerrit directly.
Currently, my origin points to gerrit (Remote Push URL
git.eclipse.org:29418/ocl/org.eclipse.ocl.git)
I don't know how this internally works, but my feelings are that
we only have one remote repo. So a normal push will always udpate
our unique GIT repo. The "push to gerrit" pushes to a different
refs (refs/for rather than refs/heads) which in somehow are not
exposed in the remote repo (nobody can fetch those refs, when
fetching from the repository).
The URL is the same, what changes is the refspec in the "push"
configuration. This is the same as what's behind the "push" and
"push to gerrit" buttons of EGit (at least, I think it is the same)
:
- git push = git push origin HEAD:refs/heads/master => push
to remote repository, bypassing gerrit
- git push origin HEAD:regs/for/master => push to gerrit
The "new remote" we name "review" in the wiki page I linked above
is no more than a shorthand for the second of these two (with
"origin" being a shortcut itself for the URI of your repository).
You could use something like this for example :
- git config remote.origin.push HEAD:refs/for/master
- This tells your "git push" and the EGit's "push" button to
push a review instead of on the remote repository
- git config remote.origin.push HEAD:refs/heads/master
Let's
see if I finally complete a Gerrit contribution to see how the
repository looks like. Meanwhile I'll login to build.eclipse.org
to have a look via command line.
"login to build.eclipse.org to have a look via command line" ? This
really scares me as to what you're going to do :p.
Cheers,
Laurent Goubet
Obeo
Cheers,
Adolfo.
--
Laurent Goubet
Obeo
_______________________________________________
mdt-ocl.dev mailing list
mdt-ocl.dev@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/mdt-ocl.dev
_______________________________________________
mdt-ocl.dev mailing list
mdt-ocl.dev@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/mdt-ocl.dev
|
