[Development] abandoning stale changes on gerrit

[Alan Alpert]

On Tue, Jan 29, 2013 at 10:43 AM, Rutledge Shawn
<Shawn.Rutledge at digia.com> wrote:
>
> On 29 Jan 2013, at 1:41 PM, Sorvig Morten wrote:
>
>>
>> On Jan 29, 2013, at 1:05 PM, Oswald Buddenhagen <oswald.buddenhagen at digia.com> wrote:
>>
>>> moin *,
>>>
>>> 5.0 is out and the 5.1 feature freeze isn't that far off any more.
>>> seems like the best time for some serious house cleaning.
>>> therefore i'd like to urge everyone to give their pending changes which
>>> haven't seen activity for a long time a honest look.
>>> please explicitly mark the ones you still want to work on by adding a
>>> comment. everything which has no indication of (planned) activity in a
>>> few weeks will be abandoned by administrative action.
>>> if you are an ex-troll/-nokian, please also check the dashboard of your
>>> alter ego.

I'm not opposed to the idea, but can we have a better implementation?
How are you defining "activity", and how old must the "activity" be
for it to be affected? Without those answers, I'll need to add an
explicit "ossi, don't touch this one" comment to every change on my
dashboard (and I will...).

I would assume it's anything with an "Updated" field in last year, but
there will be strife if anyone's assumptions are more lenient than the
actions which will be taken at an unspecified point in the future.

>>> if you are a maintainer, give identified drive-by contributors a ping.
>>> i'd also like to encourage everyone to adopt orphaned changes they have
>>> an interest in.
>>
>>
>> Please don't abandon my changes, I prefer managing the list myself.
>
> I agree; I will not like being disrupted this way.
>
> 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.

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.
https://codereview.qt-project.org/#change,38433 - Seems to have been
replaced by a new implementation already
(https://codereview.qt-project.org/#change,46020)
https://codereview.qt-project.org/#change,39624 - Nothing is, or will,
happen to this patch until we have a proper Window module discussion,
which could also lead to a different implementation. We can unabandon
it then.

I don't see what the problem is for these to be marked abandoned until
work resumes on them. Unless you're using your gerrit dashboard as a
todo list, in which case you need to learn to use JIRA for that. But
just like JIRA, we need to have some consistency in the meaning of
item states or no-one will know what is happening.

> New stuff that doesn't make it for 5.1 is still eligible for 5.2 anyway; that's what comes out of having time-based releases, and that's the point of having an ongoing dev branch too.
>
> I also don't pay attention to anyone else's clutter.  I understand that some people receive too many review requests, but here are some ideas to deal with that:
>
> 1) At least they are sorted by date so you can pay more attention to the newer ones
Are you saying that we should just ignore things past a certain
threshold date... as if they were abandoned?

> 2) If a patch is being neglected due to inactivity but still needs to be reviewed, the author can always ping you on IRC.  So ignoring inactive requests is not necessarily bad.
The author cannot always ping you on IRC. Not everyone is always on
IRC, and not everyone can match codereview names to IRC nicks. So this
might work for Thiago, but not for the project as a whole.

> 3) You can remove yourself as a reviewer to get it off your list and stop getting emails
Then the patch gets unabandoned and you don't notice. If you abandon a
patch now, and resume work (unabandon) a patch in a months time, the
reviewer *really* wants an email notification.

> 4) You can nag the author and ask that it either be abandoned or have you removed as a reviewer
ossi just did this on behalf of all reviewers, it's more efficient ;)
(removing as a reviewer is not an option).

> 5) Maybe we could add an easy nag button for that in gerrit
We could see more comments of "Work on it or mark as abandoned". But
we still need to clarify the "policy" of what abandoned means, because
if I add a comment to all those changes nagging you to abandon this,
then you aren't going to because you disagree with me about the
meaning of "abandoned".

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.

> 6) Have gerrit "expire" the reviewers on inactive patches, without abandoning the patch.  Extra points for having gerrit remember that this happened so it's easy for the author to re-add the reviewer when it's really ready.
And gerrit would also email the reviewers on activity? So how is this
different from abandoning, aside from the owner still having it show
in their dashboard?

> 7) Add filters to gerrit, to hide patches that someone has already reviewed (hide anything with -1, -2 or +2), hide patches that are too old, just show the top 20 or whatever your personal threshold is.  (kindof like writing an email client)

You could do this to help people filter out the things they're
interested in, but that's a separate discussion to the meaning of
"abandoned". We'd need a filter for the "effectively abandoned"
criteria anyways, and gerrit already has an abandoned state, so maybe
the easiest way to implement this is the force-abandon script anyways.

Also try looking at it from the other perspective - we could add a
filter for you that shows your abandoned patches for easy tracking :)
.

--
Alan Alpert