[openstack-dev] [tripleo] patch abandoment policy
Alex Schultz
aschultz at redhat.com
Mon Mar 27 14:30:35 UTC 2017
On Mon, Mar 27, 2017 at 6:47 AM, Dan Prince <dprince at redhat.com> wrote:
> On Mon, 2017-03-27 at 13:49 +0200, Flavio Percoco wrote:
>> On 24/03/17 17:16 -0400, Dan Prince wrote:
>> > On Thu, 2017-03-23 at 16:20 -0600, Alex Schultz wrote:
>> > > Hey folks,
>> > >
>> > > So after looking at the backlog of patches to review across all
>> > > of
>> > > the
>> > > tripleo projects, I noticed we have a bunch of really old stale
>> > > patches. I think it's time we address when we can abandon these
>> > > stale
>> > > patches.
>> > >
>> > > Please comment on the proposed policy[0]. I know this has
>> > > previously
>> > > been brought up [1] but I would like to formalize the policy so
>> > > we
>> > > can
>> > > reduce the backlog of stale patches. If you're wondering what
>> > > would
>> > > be abandoned by this policy as it currently sits, I have a gerrit
>> > > dashboard for you[2] (it excludes diskimage-builder) .
>> >
>> > I think it is fine to periodically review patches and abandon them
>> > if
>> > need be. Last time this came up I wasn't in fan of auto-abandoning
>> > though. Rather I just made a pass manually and did it in fairly
>> > short
>> > order. The reason I like the manual approach is a lot of ideas
>> > could
>> > get lost (or silently ignored) if nobody acts on them manually.
>> >
>> > Rather then try to automate this would it serve us better to add a
>> > link
>> > to your Gerrit query in [2] below to highlight these patches and
>> > quickly go through them.
>>
>> I used to do this in Glance. I had 2 scripts that ran every week. The
>> first one
>> would select the patches to abandon and comment on them saying that
>> the patches
>> would be abandoned in a week. The second script abandoned the patches
>> that had
>> been flagged to be abandoned that were not updated in a week.
>
> I don't think a week is enough time to react in all cases though. There
> could be a really good idea that comes in, gets flagged as abandoned
> and then nobody thinks about it again because it got abandoned.
>
So this is a different problem as it relates to the proposed patches.
People shouldn't be letting their patches go this stale. It'd be one
thing if we were doing it on a weekly basis, but the problem is that
the patches would already be 80 days stale (assuming we automated a 10
day warning) and then would get abandoned after 90 days of inactivity.
Most of the patches that fall under this policy are already so out of
date the question becomes how much are we really saving by letting
them stick around? Many times trying to rebase something this out of
date can be more of an effort than fixing it again. Who knows maybe
it was already incidentally fixed by other patches of 90 days.
Additionally if someone knows they want to come back to it, they
should mark it -1 WIP rather than just letting it sit idle. That would
give them 180 days to get back to it. The authors need to take
responsibility for their patches and not just throw things out there
and walk away.
> There is sometimes a fine line between automation that helps humans do
> their job better... and automation that goes to far. I don't think
> TripleO or Glance projects have enough patch volume that it would take
> the core team more than an hour to triage patches that need to be
> abandoned. We probably don't even need to do this weekly. Once a month,
> or once a quarter for that matter would probably be fine I think.
>
So are you signing up to triage every week? The problem with
rejecting automation is that it's adding work to an already overloaded
bunch of people who aren't able to keep up on current reviews let
alone including triaging stale reviews. I was thinking about what you
said previously about patches getting lost. There's some truth to that
but it's also exaggerating what would happen. Patches that reference
blueprints or bugs aren't lost because the abandon notice also gets
posted to launchpad. Meaning if someone hits the bug again, the patch
can be restored and reused. If as a team we actually track our
patches correctly via bugs and blueprints nothing gets lost. The
policy simply allows for the use of automation. No one is proposing
that we setup automation right now and I would agree that many of your
concerns are legitimate and would need to be addressed if we put
automation in. I would just like to allow the use of it as part of
this process even if it's a script run by a person as opposed to an
automated job.
Thanks,
-Alex
> Dan
>
>>
>> It was easy to know what patches needed to be checked since these
>> script ran w/
>> its own user (Glance Bot). I believe this worked pretty well and the
>> Glance team
>> is now working on a better version of that bot.
>>
>> I'd share the scripts I used but they are broken and depend on
>> another broken
>> library but you get the idea/rule we used in Glance.
>>
>> Flavio
>>
>> _____________________________________________________________________
>> _____
>> OpenStack Development Mailing List (not for usage questions)
>> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubs
>> cribe
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
> __________________________________________________________________________
> 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