Andrew,
The only issue I have with your proposal is that there is no way to
implement it. Gerrit auto-abandon doesn't have a way to detect 'T'.
While I agree with your idea that 'T' should be based on a time other
than the initial patch upload, I don't see a mechanism to do so. It
would appear to me that the only other solution would be to make the
window larger and anything up to the release cycle duration (120 days)
seems reasonable to me.
The other point I would make is that auto-abandon is not deletion. Thus
it puts more ownership on the patch creator to poke the maintainer(s)
for a review. And I'm assuming that 'Restore' resets the clock on the
auto-abandon trigger.
Lastly, I have not heard a counter-proposal from either you or Ole on
how to clean up the current state of the queue. We're wasting cycles
and abusing the infra by letting the queue remain so large and it would
be wise to address that sooner rather than later.
Thanks,
-daw-
On 1/28/2021 4:29 AM, Andrew 👽 Yourtchenko wrote:
On 28 Jan 2021, at 10:10, Ole Troan <otr...@employees.org> wrote:
My impression is that there is a disconnect between someone putting something
on gerrit and a vpp maintainer reviewing and contributor merging.
Absolutely agree on that.
As a project we certainly can do better on managing the stream of changes. With
my release manager hat on, I have seen a non-trivial number of cases where the
patch sits for a while, then “oh damn it’s release time”, it gets squeezed in
last moment, with predictable consequences.
I was thinking of having a script processing the review queue and generating
reports for each maintainer. Then give each author a chance to get their code
reviewed and merged.
if the maintainers don’t look at the new changes today, why would they look at
the reports on the changes they didn’t look at ?
I would like to try the gentle nudge first. If we go down the abandon route,
certainly not without sending alerts first.
If a person is legit swamped, the robotic nags will just annoy them and will go
into recycle bin. I kinda like the idea of nudges but to have a chance to help
- they need to go to the mail list where the other people might jump in... (of
course the code owners have the final say but often there can be basic things
that can be done. Maybe even do it over the email altogether - there was a
recent experience on the list and I think it might not be a bad idea to make it
a more frequent occurrence...
Then a possible sequence could be:
T: author submits the patch without their own “-1/-2” or removed -1/-2
T+7: the mail to vpp-dev is sent
T+30: autoabandon plugin triggers if no activity on the patch
—a
So a tentative -1.
Cheers
Ole
On 28 Jan 2021, at 09:19, Benoit Ganne (bganne) via lists.fd.io
<bganne=cisco....@lists.fd.io> wrote:
+1
ben
-----Original Message-----
From: vpp-dev@lists.fd.io <vpp-dev@lists.fd.io> On Behalf Of Dave Wallace
Sent: mercredi 27 janvier 2021 22:50
To: vpp-dev@lists.fd.io
Subject: [vpp-dev] RFC: Enabling Gerrit Auto-Abandon job on VPP master
Folks,
There are currently 636 open Gerrit Reviews on VPP master [0], the oldest
being submitted on Jun 13, 2016 [1]!
I would like to propose that the Gerrit Review Auto-Abandon job [2] to
reduce the size of the queue to a more reasonable length. Benefits include
(from [3]):
----- %< -----
Abandoning old inactive changes has the following advantages:
it signals change authors that changes are considered outdated
it keeps dashboards clean
it reduces the load on the server (for open changes the mergeability
flag is recomputed whenever a change is merged)
If a change is still wanted it can be restored by clicking on the Restore
button.
----- %< -----
I would like to propose the following configuration [2] for auto-abandon:
changeCleanup.abandonAfter: 30d
changeCleanup.abandonIfMergeable: default (true)
changeCleanup.cleanupAccountPatchReview: default (false)
changeCleanup.abandonMessage: default
changeCleanup.startTime: Sat 00:00
changeCleanup.interval: 1 day
If you are opposed to the use of Auto-abandon, please propose an
alternative method to clean up the backlog of reviews on VPP master and
maintain a reasonably sized queue.
If you approve of the concept, please respond with a +1.
If you approve of the concept but don't like the configuration, please
respond with your preferred configuration.
Lack of response will be interpreted as approval of the use of auto-
abandon with the proposed configuration ;)
Thanks,
-daw-
[0] dwallacelf@daw-server-2:~$ ssh -p 29418 gerrit.fd.io gerrit query
status:open project:vpp branch:master --format=JSON --no-limit | tail -1
{"type":"stats","rowCount":636,"runTimeMilliseconds":1467,"moreChanges":fa
lse}
[1] https://gerrit.fd.io/r/c/vpp/+/1529
[2] https://gerrit-review.googlesource.com/Documentation/config-
gerrit.html#changeCleanup
[3] https://gerrit-review.googlesource.com/Documentation/user-change-
cleanup.html#auto-abandon
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#18619): https://lists.fd.io/g/vpp-dev/message/18619
Mute This Topic: https://lists.fd.io/mt/80169540/21656
Group Owner: vpp-dev+ow...@lists.fd.io
Unsubscribe: https://lists.fd.io/g/vpp-dev/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
