diff --git a/bugbot/rules/uplift_beta.py b/bugbot/rules/uplift_beta.py
index cb86d66a1..911845a68 100644
--- a/bugbot/rules/uplift_beta.py
+++ b/bugbot/rules/uplift_beta.py
@@ -19,6 +19,10 @@
# too, otherwise they would all be nagged a second time.
LEGACY_COMMENT_MARKER = ", is this bug important enough to require an uplift?"
+# `fix-optional` means release management would take a fix but won't chase it,
+# so it deserves the same question as `affected`.
+AFFECTED_STATUSES = ["affected", "fix-optional"]
+
class UpliftBeta(BzCleaner):
def __init__(self):
@@ -31,11 +35,13 @@ def __init__(self):
self.versions["central"], "status", "central"
)
self.status_beta = utils.get_flag(self.beta, "status", "beta")
+ self.approval_beta = utils.get_flag(self.beta, "approval", "beta")
- # The needinfo mentions ESR generically, so we only need the current
- # ESR's status flag to tell whether ESR is affected.
+ # The needinfo mentions ESR generically, so the current ESR's flags are
+ # enough to tell whether an ESR uplift is still to be decided.
self.esr = self.versions["esr"]
self.status_esr = utils.get_flag(self.esr, "status", "esr")
+ self.approval_esr = utils.get_flag(self.esr, "approval", "esr")
# Bugs will be added to `extra_ni` later after being fetched
self.extra_ni = {
@@ -44,7 +50,7 @@ def __init__(self):
}
def description(self):
- return "Bugs fixed in nightly but still affecting beta"
+ return "Bugs fixed in nightly but still affecting beta or ESR"
def has_assignee(self):
return True
@@ -53,7 +59,35 @@ def get_extra_for_needinfo_template(self):
return self.extra_ni
def columns(self):
- return ["id", "summary", "assignee"]
+ return ["id", "channels", "summary", "assignee"]
+
+ def get_channels_to_uplift(self, bug):
+ """Get the channels the patch still needs an uplift decision for.
+
+ A channel qualifies when it is affected and nobody has asked for
+ approval on it yet. The query only guarantees that one of them is
+ affected, so this is also where the ESR-only case (beta wontfix, ESR
+ still affected) gets picked up.
+ """
+ requested_approvals = {
+ flag["name"]
+ for attachment in bug["attachments"]
+ for flag in attachment["flags"]
+ }
+
+ channels = []
+ if (
+ bug.get(self.status_beta) in AFFECTED_STATUSES
+ and self.approval_beta not in requested_approvals
+ ):
+ channels.append("beta")
+ if (
+ bug.get(self.status_esr) in AFFECTED_STATUSES
+ and self.approval_esr not in requested_approvals
+ ):
+ channels.append("ESR")
+
+ return channels
def handle_bug(self, bug, data):
bugid = str(bug["id"])
@@ -68,9 +102,9 @@ def handle_bug(self, bug, data):
if self.is_needinfo_on_assignee(bug.get("flags", []), assignee):
return None
- # Flag ESR using the same criteria as beta (see get_bz_params): both
- # "affected" and "fix-optional" should prompt about an uplift.
- esr_affected = bug.get(self.status_esr) in ("affected", "fix-optional")
+ channels = self.get_channels_to_uplift(bug)
+ if not channels:
+ return None
data[bugid] = {
"id": bugid,
@@ -78,7 +112,7 @@ def handle_bug(self, bug, data):
"nickname": nickname,
"summary": self.get_summary(bug),
"regressions": bug["regressions"],
- "esr_affected": esr_affected,
+ "channels": channels,
}
return bug
@@ -121,15 +155,16 @@ def is_needinfo_on_assignee(self, flags, assignee):
def get_bz_params(self, date):
self.date = lmdutils.get_date_ymd(date)
fields = [
- self.status_beta,
- self.status_esr,
"regressions",
"attachments.creation_time",
"attachments.is_obsolete",
"attachments.content_type",
+ "attachments.flags",
"cf_last_resolved",
"assigned_to",
"flags",
+ self.status_beta,
+ self.status_esr,
]
params = {
"include_fields": fields,
@@ -138,30 +173,36 @@ def get_bz_params(self, date):
"f1": self.status_central,
"o1": "anyexact",
"v1": ",".join(["fixed", "verified"]),
- "f2": self.status_beta,
- "o2": "anyexact",
- "v2": ["affected", "fix-optional"],
- "f3": "flagtypes.name",
- "o3": "notsubstring",
- "v3": "approval-mozilla-beta",
# Don't nag several times
- "n5": 1,
- "f5": "longdesc",
- "o5": "casesubstring",
- "v5": COMMENT_MARKER,
+ "n2": 1,
+ "f2": "longdesc",
+ "o2": "casesubstring",
+ "v2": COMMENT_MARKER,
# Same, for bugs nagged with the previous wording
- "n8": 1,
- "f8": "longdesc",
- "o8": "casesubstring",
- "v8": LEGACY_COMMENT_MARKER,
+ "n3": 1,
+ "f3": "longdesc",
+ "o3": "casesubstring",
+ "v3": LEGACY_COMMENT_MARKER,
# Check if have at least one attachment which is a Phabricator request
- "f6": "attachments.mimetype",
- "o6": "anyexact",
- "v6": ["text/x-phabricator-request", "text/x-github-pull-request"],
+ "f4": "attachments.mimetype",
+ "o4": "anyexact",
+ "v4": ["text/x-phabricator-request", "text/x-github-pull-request"],
# skip if whiteboard contains checkin-needed-beta (e.g. test-only uplift)
- "f7": "status_whiteboard",
- "o7": "notsubstring",
- "v7": "[checkin-needed-beta]",
+ "f5": "status_whiteboard",
+ "o5": "notsubstring",
+ "v5": "[checkin-needed-beta]",
+ # Beta or ESR must be affected. Which of them still needs a
+ # decision is worked out in get_channels_to_uplift(), where we can
+ # look at the approval requests channel by channel.
+ "j6": "OR",
+ "f6": "OP",
+ "f7": self.status_beta,
+ "o7": "anyexact",
+ "v7": AFFECTED_STATUSES,
+ "f8": self.status_esr,
+ "o8": "anyexact",
+ "v8": AFFECTED_STATUSES,
+ "f9": "CP",
}
return params
@@ -174,7 +215,7 @@ def get_bugs(self, date="today", bug_ids=[]):
if data["mail"] and data["nickname"]:
self.extra_ni[bugid] = {
"regression": len(data["regressions"]),
- "esr_affected": data["esr_affected"],
+ "channels": data["channels"],
}
self.add_auto_ni(
bugid, {"mail": data["mail"], "nickname": data["nickname"]}
diff --git a/templates/uplift_beta.html b/templates/uplift_beta.html
index f4cb48399..9bec762da 100644
--- a/templates/uplift_beta.html
+++ b/templates/uplift_beta.html
@@ -1,22 +1,24 @@
- The following {{ plural('bug has', data, pword='bugs have') }} been fixed in nightly and {{ plural('is', data, pword='are') }} affecting beta. {{ plural('Assignee', data) }} got a needinfo to ask if it's worth uplifting the patches:
+ The following {{ plural('bug has', data, pword='bugs have') }} been fixed in nightly and {{ plural('is', data, pword='are') }} still affecting beta or ESR. {{ plural('Assignee', data) }} got a needinfo to ask if it's worth uplifting the patches:
| Bug |
+ Channels to uplift to |
Summary |
Assignee |
- {% for i, (bugid, summary, assignee) in enumerate(data) -%}
+ {% for i, (bugid, channels, summary, assignee) in enumerate(data) -%}
|
{{ bugid }}
|
+ {{ channels | join(", ") }} |
{{ summary | e }} |
{{ assignee | e }} |
diff --git a/templates/uplift_beta_needinfo.txt b/templates/uplift_beta_needinfo.txt
index 19d5ba78f..0e4600b5c 100644
--- a/templates/uplift_beta_needinfo.txt
+++ b/templates/uplift_beta_needinfo.txt
@@ -1,13 +1,12 @@
-The patch landed in nightly and beta is affected{% if extra[bugid]["esr_affected"] %}, along with ESR{% endif %}.
+The patch landed in nightly and {{ extra[bugid]["channels"][0] }} is affected{% if extra[bugid]["channels"] | length > 1 %}, along with ESR{% endif %}.
-:{{ nickname }}, {{ extra["question"] }} {% if extra[bugid]["esr_affected"] %}each affected release{% else %}beta{% endif %}:
-{% if extra[bugid]["esr_affected"] -%}
+:{{ nickname }}, {{ extra["question"] }} {{ extra[bugid]["channels"] | join(" and ") }}:
+{% if "beta" in extra[bugid]["channels"] -%}
- For **beta**, nominate the patch for beta uplift approval if the fix should be included in this release, or set `{{ extra["status_beta"] }}` to `wontfix` if the fix can wait for the next release.
+{% endif -%}
+{% if "ESR" in extra[bugid]["channels"] -%}
- For **ESR**, nominate the patch for the appropriate ESR uplift approval if the fix should be included in ESR, or set the ESR status flag(s) to `wontfix` if an uplift isn't needed. Each affected ESR release needs a decision.
-{%- else -%}
-- Nominate the patch for beta uplift approval if the fix should be included in this release, or
-- Set `{{ extra["status_beta"] }}` to `wontfix` if the fix can wait for the next release.
-{%- endif %}
+{% endif -%}
{% if extra[bugid]["regression"] %}
Don't forget to request an uplift for the patches in the {{ extra[bugid]["regression"] }} {{ plural('regression', extra[bugid]["regression"]) }} caused by this fix.
{% endif %}