-
Notifications
You must be signed in to change notification settings - Fork 13
Scan and un-verify outdated Gerrit changes #63
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
92b0666
to
abd198e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So in general all this LGTM. That said, I wonder if we could maybe extend a bit in the future to compare the timestamp of the change against the latest committer date of the master branch? The scenario I am thinking about here is this:
- change was not updated for 4 weeks
- master was not updated for 4 weeks
- the fact that change is "outdated" is not relevant since last master commit is as old as the change (so we shouldn't really vote -1 here)
This of course is still not checking the "stability" of the change but paints a bit clearer picture I guess. With that in mind we could also see if given change is still mergeable or not. This could be done as an extra step to also indicate that "ok this is old and definitely not suitable for merge, rebase!")
More or less I would look up this:
In [13]: a = json.loads(requests.get("https://review.spdk.io/projects/spdk%2Fspdk/branches/master").text.splitlines()[1])
In [14]: a['revision']
Out[14]: 'c779c57a6e9947bc8e7987b63f7af1586e481485'
In [15]: b = json.loads(requests.get(f"https://review.spdk.io/projects/spdk%2Fspdk/commits/{a['revision']}").text.splitlines()[1])
In [16]: b['committer']['date']
Out[16]: '2025-05-19 07:22:29.000000000' # compare change ts against this
In [17]:
and this:
In [8]: json.loads(requests.get(f"https://review.spdk.io/changes/{change_id}/revisions/current/mergeable").text.splitlines()[1])["mergeable"]
Out[8]: True #
Food for thought. :)
This workflow will query Gerrit and look for changes which were not updated for a while. If there was no activity for: * 2 weeks - the script will suggest user to rebase their change * 4 weeks - the script will set Verified -1 vote on change, and ask user to rebase. Workflow runs daily as 12AM UTC, or can be run on-demand by priviledged users. There's also an additional 12 weeks cut-off date. Changes older than that will NOT be commented on to avoid polluting Gerrit dashboards with very old changes (e.g. 2-3 years old). Signed-off-by: Karol Latecki <[email protected]>
Noted, thanks @mikeBashStuff. I'll try adding this to the PR if it doesn't get merged until then. |
abd198e
to
116f9c1
Compare
As in other workflows and services - ignore "work in progress" changes. Signed-off-by: Karol Latecki <[email protected]>
Check change's age versus it's branch tip, not the absolute age. If there's not much (or anything at all) going on in the branch then there's no point in bugging the owner. Signed-off-by: Karol Latecki <[email protected]>
Datetime formatting line is a pretty long one, so move it to a separate function and narrow down imports to reduce it's length. Signed-off-by: Karol Latecki <[email protected]>
@mikeBashStuff I pushed changes with checking change age vs branch tip. Can you take a look? |
This workflow will query Gerrit and look for changes which were not updated for a while. If there was no activity for:
Workflow runs daily as 12AM UTC, or can be run on-demand by priviledged users.
There's also an additional 12 weeks cut-off date. Changes older than that will NOT be commented on to avoid polluting Gerrit dashboards with very old changes (e.g. 2-3 years old).