[Development] [FYI] new git-gpush features, a.k.a. the smart way of pushing to gerrit

[Oswald Buddenhagen]

On Fri, Oct 24, 2014 at 12:28:42PM +0200, Kalinowski Maurice wrote:
> Furthermore one of the concerns is to change the workflow with git
> compared to other projects beside Qt. After all, it is different
> commands to be used when pushing to Qt gerrit if your proposal is
> accepted. So the benefits clearly have to outweigh those process
> changes.
> 
the changes are trivial, and should be actually welcomed by newbies due
to offering a less scary command line.
furthermore, these scrips are in no way specific to qt. anyone can use
them with any gerrit installation. i'll propose them for the contrib/
dir of upstream gerrit when they are considered mature enough.

> IIRC Thiago has been one of persons using it in the past, has it been
> seen beneficial or causing problems? What is the impression by others
> who have actually tested it so far?
> 
i don't think the number of qt people *capable* of using thiago's script
exceeds half a dozen, and those *willing* to use it one. ^^

On Fri, Oct 24, 2014 at 11:47:21AM +0000, Smith Martin wrote:
> I speak for all the people who play "Myst" by just walking around the
> island enjoying the views and listening to the waves.
> 
you are consistently asking for easy solutions for hard problems.
luckily, these scripts are *just right for you*.

On Fri, Oct 24, 2014 at 12:38:29PM +0200, Jędrzej Nowacki wrote:
> I think you took the wrong way. You are fixing the problem on odd
> level,
>
i don't think so. some of the problems could be fixed on the server
side, but others are inherently client-side.

> enforcing a tool on everyone is at the best far from ideal.
> 
we already do that by requiring the change-id hook.
in fact, this script suite could be used to ease this requirement.

> "don't create spurious dependencies" -> I didn't even know that it is a 
> problem.
>
> Especially that in most cases a change is staged by its author, which 
> is supposed to know the real dependency.
>
as most of the issues this script addresses, it's about the reviewers.
the most obvious point is that spurious dependencies are ugly and
confusing.
but they get really annoying when you re-push changes just because they
happend to be still in your local branch and you modified entirely
unrelated changes. it would be possible to hack around this to some
degree with server-side heuristics, but solving the problem closer to
the source is a cleaner approach.

On Fri, Oct 24, 2014 at 12:55:40PM +0200, Olivier Goffart wrote:
> > "i pushed an update to your change, take care not to overwrite it
> > accidentally"
> 
> This is actually something that would be worth to enforce.
> How is gpush detecting that? 
>
by comparing the current state of gerrit with what you pushed the last
time. https://codereview.qt-project.org/96299

> Can it not be enforced with a normal git push? 
> 
no. it's an inherently client-side thing. (it would be possible to do
what the script does if there was a post-push hook in addition to
pre-push. but then you have to install additional hooks, which seems
much more complicated than simply using a script.)

On Fri, Oct 24, 2014 at 11:15:24AM +0000, Rutledge Shawn wrote:
> I’ve always thought the “don’t rebase” advice is artificial, because I
> prefer not to try to integrate a patch until I’ve made sure that it
> can be rebased on top of the latest code, and still works.
> 
funny you should mention it. as it happens, it's one of the observations
that led to my work.

> So it’s often a multi-step process when revising a patch:
> [complicated procedure follows]
>
such a coincidence ... it's *exactly* what the script does, without
you needing to worry about any of it and paying any price (except
waiting a few more seconds compared to a direct push).

you are entirely correct that gerrit could do _that part_ server-side.
however, to address only the inter-diff issue, christian's approach is
potentially better, as it is more fine-grained. however, it is also much
slower on the reviewer side (did you notice that inter-diffs with big
rebases are already rather slow? it would be about twice as slow).

but the script offers many more features, some of which are either not
implementable on the server at all, or would need a client-side frontend
to be useful anyway.

> And besides, when reviewing it’s better to give the entire patch one
> more look before giving +2, to make sure that the whole thing still
> makes sense in context.
>
uhm, so? this is about optimizing some aspects of the workflow, not
forbidding others.

> Now you will just brag again about how your tools are better than
> whatever anyone else can possibly ever think of,
>
no, i'll point out that i'm not aware of any other solutions that
comes even close to what i have done.

> and make more of your usual ad-hominem accusations.
>
*that* is an ad-hominem, quite contrary to anything i uttered in this
thread (hint: it's not ad-hominem when it is pertinent to the matter at
hand).

> But your dictatorial attitude leaves a bad taste and actually does not
> make me want to try them at all, compared to the mail a couple of days
> ago where you were just pitching them on the basis of usefulness.
> 
did it occur to you that this footnote was a response to feedback i
already got?

also, this isn't a democracy. i maintain a good part of our
gerrit-related infrastructure. if you have specific concerns about the
solution i came up with, bring them up, and they'll get due
consideration. but don't complain about not having been asked if you
don't actually have anything to criticise. especially when the time of
implementation was announced to be "at some point" (that doesn't exactly
sound like "next week", does it?).