[Development] abandoning stale changes on gerrit

[Alan Alpert]

On Thu, Jan 31, 2013 at 2:58 AM, Rutledge Shawn
<Shawn.Rutledge at digia.com> wrote:
> On 29 Jan 2013, at 8:43 PM, Oswald Buddenhagen wrote:
>
>> On Tue, Jan 29, 2013 at 06:43:34PM +0000, Rutledge Shawn wrote:
>>> I do actually abandon stuff when it's quite clear that it's dead, but
>>> due to the review and CI processes, there's quite a large percentage
>>> of what I write that has hit some sort of obstacle and yet is still a
>>> good idea to somehow get done.
>>>
>> before somebody gets stupid ideas:
>
> What is this, ad hominem?
>
> I question your right to unilaterally decide to put the kibosh on other people's patches, automatically, across the whole project, even when they are not on your own review list.  You've had enough feedback from others too about _that_ idea.
>
>
> On 29 Jan 2013, at 8:37 PM, Alan Alpert wrote:
>> We're not talking about killing any changes. We're talking about
>> making the state in the system match reality - that change has been
>> abandoned in practice even if you don't like to face that harsh
>> reality. Abandoned is not -2, you can reopen it when you want to do
>> something with it again.
>>
>> To look at some concrete examples, here are several of your changes
>> which I'm a reviewer on which haven't moved since November:
>> https://codereview.qt-project.org/#change,38313 - Good idea, still
>> needs to be done, but this patch is going nowhere and it could just be
>> replaced when (if ever) the new implementation gets pushed.

The content of those changes doesn't really matter (in this thread -
in their own thread the content is super important). The point I was
trying to make is that there are specific commits which I think
auto-abandoning would be appropriate for, and it appears you disagree
(except for the one which was manually abandoned).

> It took some time to write and test, and could have already gone in if it wasn't for the trouble with the debugging interface.  If I was going to replace it, I probably wouldn't start again from scratch.  If you want to fix it yourself, I wouldn't complain either.  ;-)

(Side point: the trouble isn't with the debugging interface, it's with
a primary usecase) Abandoning a change doesn't mean the next one needs
to start from scratch. You can either un-abandon it, or cherry-pick
the old change to start your new one.

If I wanted to fix it myself, chances are that I'd go to the JIRA
task. If the task mentions a change, or I know that someone once
worked on this, I would search gerrit for commit messages containing
that JIRA task. Even if that was in my review queue on the dashboard,
I wouldn't find it that way in that case. And if someone else found
the JIRA task and wanted to fix it, then the state of the change would
certainly not affect whether they find it or not.

>> https://codereview.qt-project.org/#change,38433 - Seems to have been
>> replaced by a new implementation already
>
> abandoned now
>
>> (https://codereview.qt-project.org/#change,46020)
>
> You are the only one who doesn't like that patch.  Otherwise it could have gone in before Qt 5.0.

For the sake of argument, let's assume that I'm an obstinate,
cantankerous troll blocking the path of progress. That patch has still
be effectively abandoned, because no progress has been made to
overcome that objection. Either it's in a deadlock going nowhere
(effectively abandoned), or the objection has been successfuly (and it
should be made abandoned).

If an objection is raised and nothing more happens with the patch,
there's no reason to believe things will magically start to happen
again at some point - this is practically the definition of abandoned.
You need to work on resolving the conflict (in this case, we'll
discuss the matter off-line before the 5.1 freeze), just like how you
need to work on resolving auto-test failures before a change can be
merged.

>
>> https://codereview.qt-project.org/#change,39624
>
> This patch is the opposite of the previous one, so only one will go in.  But why bring that discussion into this one?  I'm not going to abandon either one of them until one of them is integrated.

This is a confusing gerrit state. You have two patches both marked for
review, but one of them isn't worth reviewing because it won't go in.
If they both reference each other in the comments (they don't) then
that would be a more acceptable state. But I think the clearest state
is for you to keep your preferred change and abandon the other. If the
preferred change gets shot down, you can always unabandon the other
one and resume reviews on it. But is it really fair for you to ask
reviewers to consider the two equally, when you only intend for one of
them to actually get through?

>> I don't see what the problem is for these to be marked abandoned until
>> work resumes on them.
>
> Because they become hard to find.
>
>> Unless you're using your gerrit dashboard as a
>> todo list, in which case you need to learn to use JIRA for that.
>
> We all have our own ways of remembering things, so I don't think you should be saying that one of them is wrong if it's actually helpful.
>

Unfortunately, because we're all working together on the same large
project it is not helpful to disregard conventions. When the old Qt3D
left, new contributors wanted to take over that work because it's a
really cool idea, but they did not get a guide saying how each
individual Qt3D contributor had stored his work plans. Instead they
used JIRA to find the plans and current state of work items in the
project, because that's the convention the Qt3D people should have
been following. I just checked and their dashboards still have
unfinished changes in them, but who knows what that means? Only those
people, and they've all moved on from Qt by now.

>> Also, nagging is *never* the ideal solution. For anything. So I really
>> would not like a "nag button" formalized in the process, nagging is
>> only for when the process breaks down.
>
> Somebody apparently thought the process already broke down, else this thread wouldn't have started.  But I agree nagging is usually annoying.
>
>> Also try looking at it from the other perspective - we could add a
>> filter for you that shows your abandoned patches for easy tracking :)
>
> That's a really good idea, as long as there's a guarantee that abandoned patches don't get deleted until I want them to be.  Then we could label it "deferred", at least that sounds better to me.

Abandoned changes (just like merged ones) count as history of the
project and thus should never be deleted. There are lots of different
reasons for abandoning, even if people don't always agree on them, yet
I still don't think any of them provide a valid reason for erasing the
code and reviews from the history of the project.

--
Alan Alpert