[srobo-devel] Gerrit: "rebase if necessary" on srweb

[Jeremy Morse]

Hi,

A couple of months ago Alistair suggested that the action taken when
srweb patches are submitted should be changed -- previously patches were
merged with merge commits. The alternative was to use "rebase" instead.
This trades precise information about how things were submitted for
having a straight history.

I turned this on shortly after Kickstart. If you now visualize srwebs
history (with, say, gitg), you get a completely linear history, which is
nice.

I think this works very well for srweb and we should keep it: the
history can now be read. I think for other projects it probably isn't as
useful, because srweb has a particularly special workflow. (Plus, I
think pushing up a merge commit to a project and then having it rebased
may lead to explosions).

--
Thanks,
Jeremy

[Peter Law]

Hi,

Jeremy wrote:
*we've been using rebase style patch application for a while*
*snip*

Post by Jeremy Morse
I think this works very well for srweb and we should keep it: the
history can now be read. I think for other projects it probably isn't as
useful, because srweb has a particularly special workflow. (Plus, I
think pushing up a merge commit to a project and then having it rebased
may lead to explosions).

I agree. For the standard srweb workflow this is much improved.

There are some other things I've noticed when developing a collection
of changes in a branch which are dependent on each other (as I have
been doing for the srcomp related changes).

In particular, the previous branch-merge [1] behaviour (where an
implicit branch could be merged all in one go) doesn't seem to work as
well. I also noticed that where previously failing to properly trigger
a branch-merge you could just continue and merge the later items,
you're now forced to rebase any later items regardless.

I don't think these issues are worth switching back for, they're just
worth knowing about and maybe we can look at being able to also
direct-merge branches after review, rather than rebase-applying them.

Thanks,
Peter

[1] In which you could pressed the submit button on a series of
patches in reverse-submission order (i.e: starting with the one
authored most recently) and gerrit would merge them all as a single
merge

[Jeremy Morse]

Hi,

Post by Peter Law
There are some other things I've noticed when developing a collection
of changes in a branch which are dependent on each other (as I have
been doing for the srcomp related changes).
In particular, the previous branch-merge [1] behaviour (where an
implicit branch could be merged all in one go) doesn't seem to work as
well. I also noticed that where previously failing to properly trigger
a branch-merge you could just continue and merge the later items,
you're now forced to rebase any later items regardless.
I don't think these issues are worth switching back for, they're just
worth knowing about and maybe we can look at being able to also
direct-merge branches after review, rather than rebase-applying them.

From reading the gerrit mailing list, some of the most long winded and
complex discussions are about how to go about reviewing a branch or set
of patches, and then merging it as one item. As far as I know, the
solution is that there is no solution right now. There may be one in a
couple of versions time.

One can however push up merge commit to gerrit; in combination with
being able to create branches and access non-maintained branches, we'd
then be able to develop something and then submit a merge request in the
form of a gerrit changeset (but, minus the the current review system).

The prolog rules necessary to do that (according to an email entitled
'wuwuwuwu' that I sent to myself a year ago) is:

submit_rule(S) :-
gerrit:change_branch('refs/heads/master'),
!,
gerrit:default_submit(X),
X =.. [submit | Ls],
add_category_min_score([],'Code-Review', 3, NewLabels),
S =.. [submit | NewLabels].

submit_rule(S) :-
gerrit:default_submit(X),
X =.. [submit | Ls],
add_category_min_score([],'Code-Review', 1, NewLabels),
S =.. [submit | NewLabels].

--
Thanks,
Jeremy

[Jeremy Morse]

Hi,

Post by Jeremy Morse
The prolog rules necessary to do that

After actually reading that, actually those rules just turn off
3-votes-needed for non-master branches. Unmaintained branches for
projects could be done by a more fine grained 'Maintained-Projects' ACL.

--
Thanks,
Jeremy

[Jeremy Morse]

Hi,

https://www.studentrobotics.org/gerrit/#/c/2320/

This patch appears to be a victim of this feature. The risk-assessment
publishing patch that was it's parent was submitted and merged into the
repository; however in that process the parent was rebased. This results
in a patch that is completely valid, but because the parent was rebased,
it's now out of date.

Thoughts?

--
Thanks,
Jeremy

[Jeremy Morse]

Hi,

Post by Jeremy Morse
https://www.studentrobotics.org/gerrit/#/c/2320/
This patch appears to be a victim of this feature. The risk-assessment
publishing patch that was it's parent was submitted and merged into the
repository; however in that process the parent was rebased. This results
in a patch that is completely valid, but because the parent was rebased,
it's now out of date.
Thoughts?

Just happened again with:

https://www.studentrobotics.org/gerrit/#/c/2494/

Maybe this is OK if it happens once every six months, but has anyone
else encountered this?

--
Thanks,
Jeremy

[Peter Law]

Post by Jeremy Morse
Maybe this is OK if it happens once every six months, but has anyone
else encountered this?

The other exception is heavy development of the core PHP pieces, but
that's also rare enough that the current setup is preferable.

Thank,
Peter