Skip to content

Commit 0145ed0

Browse files
Always auto-move patches tagged Bugfix when a commitfest closes (#120)
Álvaro pointed out on pgsql-hackers[1] that bug fixes get lost in closed commitfests, because the auto-move rules skip patches whose thread is quiet or whose CI has been failing for too long. Now every open patch tagged Bugfix is moved to the next commitfest when a commitfest closes, regardless of activity or CI state. [1]: https://www.postgresql.org/message-id/ar4QFqNuUX-e0v2B%40alvherre.pgsql --------- Co-authored-by: Jelterminator FennemAI-Neo <fennemai@jeltef.nl>
1 parent 1b04625 commit 0145ed0

3 files changed

Lines changed: 58 additions & 7 deletions

File tree

‎pgcommitfest/commitfest/models.py‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@
1111

1212
from .util import DiffableModel
1313

14+
# Patches with one of these tags are always moved to the next commitfest when
15+
# their commitfest closes.
16+
AUTO_MOVE_ALWAYS_TAG_NAMES = ["Bugfix"]
17+
1418

1519
# We have few enough of these, and it's really the only thing we
1620
# need to extend from the user model, so just create a separate
@@ -114,10 +118,15 @@ def to_json(self):
114118
def _should_auto_move_patch(self, patch, current_date):
115119
"""Determine if a patch should be automatically moved to the next commitfest.
116120
117-
A patch qualifies for auto-move if it both:
121+
A patch tagged as a bug fix is always moved, so that it doesn't get
122+
lost in a closed commitfest. Any other patch qualifies for auto-move
123+
if it both:
118124
1. Has had email activity within the configured number of days
119125
2. Hasn't been failing CI for longer than the configured threshold
120126
"""
127+
if any(tag.name in AUTO_MOVE_ALWAYS_TAG_NAMES for tag in patch.tags.all()):
128+
return True
129+
121130
activity_cutoff = current_date - timedelta(
122131
days=settings.AUTO_MOVE_EMAIL_ACTIVITY_DAYS
123132
)
@@ -148,8 +157,8 @@ def _should_auto_move_patch(self, patch, current_date):
148157
def auto_move_active_patches(self):
149158
"""Automatically move active patches to the next commitfest.
150159
151-
A patch is moved if it has recent email activity and hasn't been
152-
failing CI for too long.
160+
A patch is moved if it is tagged as a bug fix, or if it has recent
161+
email activity and hasn't been failing CI for too long.
153162
"""
154163
current_date = datetime.now()
155164

@@ -170,9 +179,13 @@ def auto_move_active_patches(self):
170179
).order_by("startdate")[0]
171180

172181
# Get all patches with open status in this commitfest
173-
open_pocs = self.patchoncommitfest_set.filter(
174-
status__in=PatchOnCommitFest.OPEN_STATUSES
175-
).select_related("patch")
182+
open_pocs = (
183+
self.patchoncommitfest_set.filter(
184+
status__in=PatchOnCommitFest.OPEN_STATUSES
185+
)
186+
.select_related("patch")
187+
.prefetch_related("patch__tags")
188+
)
176189

177190
for poc in open_pocs:
178191
if self._should_auto_move_patch(poc.patch, current_date):

‎pgcommitfest/commitfest/templates/help.html‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ <h2>Commitfest</h2>
1717

1818
<h3>Commitfest closure</h3>
1919
<p>
20-
When a Commitfest closes, patches that have been active recently are automatically moved to the next Commitfest. A patch is considered "active" if it has had email activity in the past {{auto_move_email_activity_days}} days and has not been failing CI for more than {{auto_move_max_failing_days}} days. Patches that are not automatically moved will stay in the closed Commitfest, where they will no longer be picked up by CI. Authors of such patches that have enabled "Notify on all where author" in their profile settings will receive an email notification asking them to either move the patch to the next Commitfest or close it with an appropriate status.
20+
When a Commitfest closes, patches that have been active recently are automatically moved to the next Commitfest. A patch is considered "active" if it has had email activity in the past {{auto_move_email_activity_days}} days and has not been failing CI for more than {{auto_move_max_failing_days}} days. Patches tagged "Bugfix" are always moved, even if they are not active. Patches that are not automatically moved will stay in the closed Commitfest, where they will no longer be picked up by CI. Authors of such patches that have enabled "Notify on all where author" in their profile settings will receive an email notification asking them to either move the patch to the next Commitfest or close it with an appropriate status.
2121
</p>
2222

2323
<h2>Patches</h2>

‎pgcommitfest/commitfest/tests/test_closure_notifications.py‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
PatchHistory,
1414
PatchOnCommitFest,
1515
PendingNotification,
16+
Tag,
1617
)
1718
from pgcommitfest.mailqueue.models import QueuedMail
1819
from pgcommitfest.userprofile.models import UserProfile
@@ -310,6 +311,43 @@ def test_no_auto_move_when_failing_too_long(alice, in_progress_cf, open_cf):
310311
assert "Failing Patch" in body
311312

312313

314+
def test_auto_move_bugfix_even_when_inactive_and_failing(
315+
alice, in_progress_cf, open_cf
316+
):
317+
"""Patches tagged Bugfix should be auto-moved even without recent email
318+
activity and with CI failing for too long."""
319+
patch = Patch.objects.create(
320+
name="Inactive Failing Bugfix",
321+
lastmail=datetime.now()
322+
- timedelta(days=settings.AUTO_MOVE_EMAIL_ACTIVITY_DAYS + 10),
323+
)
324+
patch.authors.add(alice)
325+
patch.tags.add(Tag.objects.get(name="Bugfix"))
326+
PatchOnCommitFest.objects.create(
327+
patch=patch,
328+
commitfest=in_progress_cf,
329+
enterdate=datetime.now(),
330+
status=PatchOnCommitFest.STATUS_REVIEW,
331+
)
332+
CfbotBranch.objects.create(
333+
patch=patch,
334+
branch_id=3,
335+
branch_name="test-branch-3",
336+
apply_url="https://example.com",
337+
build_url="https://example.com/build/3",
338+
status="failed",
339+
failing_since=datetime.now()
340+
- timedelta(days=settings.AUTO_MOVE_MAX_FAILING_DAYS + 10),
341+
)
342+
343+
in_progress_cf.auto_move_active_patches()
344+
in_progress_cf.send_closure_notifications()
345+
346+
patch.refresh_from_db()
347+
assert patch.current_commitfest().id == open_cf.id
348+
assert QueuedMail.objects.count() == 0
349+
350+
313351
def test_auto_move_when_failing_within_threshold(alice, in_progress_cf, open_cf):
314352
"""Patches failing CI within the threshold should still be auto-moved."""
315353
patch = Patch.objects.create(

0 commit comments

Comments
 (0)