[openstack-dev] auto-abandon changesets considered harmful (was Re: [stable][all] Revisiting the 6 month release cycle [metrics])

Doug Hellmann doug at doughellmann.com
Thu Mar 5 15:23:43 UTC 2015



On Thu, Mar 5, 2015, at 06:59 AM, Alexis Lee wrote:
> Doug Hellmann said on Wed, Mar 04, 2015 at 11:10:31AM -0500:
> > I used to use email to track such things, but I have reached the point
> > where keeping up with the push notifications from gerrit would consume
> > all of my waking time.
> 
> Jim said if his patch was auto-abandoned, he would not find out. There
> are mail filtering tools, custom dashboards, the REST API. There are a
> myriad of ways to find out, it seems false to complain about a lack of
> notification if you turned them off yourself and choose not to use
> alternative tooling.

I can certainly do all of those things, and do, but I'm becoming a more
skilled gerrit user. My concern is about new contributors, who have not
learned gerrit or who have not learned the way we have customized our
use of gerrit. Throwing away their work because they've been away for
some period of time (think extended vacation, illness, family emergency)
is hostile. The patch is already in a state that would prevent it from
merging, no? So if it we can hide it from reviewers that don't want to
see those sorts of patches, without being hostile and without hiding it
from *all* reviewers, that seems better than automatically discarding
it.

> 
> I'm not saying it's perfect. Gerrit could offer granular control of push
> notifications. It's also awkward to filter auto-abandoned patches from
> manually abandoned, which is why I think a new flag or two with bespoke
> semantics is the best solution.
> 
> > As Jim and others have pointed out, we can identify those changesets
> > using existing attributes rather than having to add a new flag.
> 
> This doesn't help new contributors who aren't using your custom
> dashboard. The defaults have to be sensible. The default dashboard must
> identify patches which will never be reviewed and a push notification
> should be available for when a patch enters that state.

Does the default dashboard we have not seem sensible for a new
contributor? When I'm logged in it shows me all of my open changes (this
is where abandoning a change makes it "disappear") and reviews where
I've commented. A slightly more advanced user can star changes and see
all of them in a view, and watch projects and see their open changes.
More advanced users can query by project or status, and even more
advanced users can bookmark those queries or even install dashboards
into gerrit.

We have different types of users, who will want different things from
gerrit. I don't think we want the default view to match some sort of
expert view, but we should make it easier for users to do more with the
tool as they gain more experience.

> 
> It also doesn't help composability. What if I want to find active
> patches with less than 100 lines of code change? I have to copy the
> whole query string to append my "delta:<100". Copying the query string
> everywhere makes it easy for inconsistency to slip in. If the guideline
> changes, will every reviewer update their dashboards and bookmarks?

I don't think every reviewer needs to be looking at the same view, so I
don't think maintaining consistency is a problem. To share that Oslo
dashboard, we have a link in the wiki and we keep it up to date by
managing the dashboard creator input file. It doesn't change often
(usually only when we start graduating a new library) so it's not a lot
of work.

> 
> Projects have demonstrated a desire to set policy on patch activity
> centrally, individual custom dashboards don't allow this. Official

We should be focusing on maximizing the ability of reviewers to find the
patches they do want to review, rather than ensuring that everyone is
reviewing the same thing. So far I have only seen discussion of
abandoning patches that wouldn't merge anyway. Reviewers who come across
those patches will see the -1 from jenkins or the -2 from a core
reviewer and either understand that means they should skip reviewing it
or waste a small amount of their time until they do learn that. Either
way reviewers who want to ignore the patch by using a filter query will
be able to continue to do that.

> custom dashboards (that live on the Gerrit server) are a pain to change
> AFAIK and we can't count on new contributors knowing how important they
> are. Allowing projects to automatically flag patches helps both those
> who read their Gerrit email and those who rely on custom dashboards.
> 
> 
> Alexis
> -- 
> Nova Engineer, HP Cloud.  AKA lealexis, lxsli.
> 
> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe:
> OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev



More information about the OpenStack-dev mailing list