Show warning when a critical PR isn't based against stable

Author: wilzbach

source/dlangbot/app.d

@@ -209,7 +209,7 @@ void handlePR(string action, PullRequest* _pr)
     UserMessage[] msgs;
     if (action == "opened" || action == "synchronize")
     {
-        //msgs = pr.checkForWarnings;
+        msgs = pr.checkForWarnings(descs);
     }
 
     pr.updateGithubComment(comment, action, refs, descs, msgs);

source/dlangbot/bugzilla.d

@@ -51,9 +51,9 @@ struct Issue
 {
     int id;
     string desc;
-    string bug_status;
+    string status;
     string resolution;
-    string bug_severity;
+    string severity;
     string priority;
 }
 

source/dlangbot/github.d

@@ -48,7 +48,9 @@ string formatComment(R1, R2)(R1 refs, R2 descs, UserMessage[] msgs)
     }
     if (msgs.length)
     {
-        app ~= "### Warnings \n\n";
+        if (refs.length)
+            app ~= "\n";
+        app ~= "### Warnings\n\n";
         app.printMessages(msgs);
     }
     return app.data;
@@ -137,6 +139,7 @@ void updateGithubComment(in ref PullRequest pr, in ref GHComment comment,
 // Github Auto-merge
 //==============================================================================
 
+        string ref_;
 alias LabelsAndCommits = Tuple!(Json[], "labels", Json[], "commits");
 enum MergeMethod { none = 0, merge, squash, rebase }
 
@@ -432,6 +435,7 @@ struct PullRequest
     static struct Branch
     {
         string sha;
+        string ref_;
         Repo repo;
     }
     Branch base, head;

source/dlangbot/warnings.d

@@ -1,10 +1,14 @@
 module dlangbot.warnings;
 
+import dlangbot.bugzilla : Issue;
 import dlangbot.github : PullRequest;
 
+import std.algorithm;
+
 struct UserMessage
 {
     enum Type { Error, Warning, Info }
+    Type type;
 
     string text;
 }
@@ -21,24 +25,35 @@ Check bugzilla priority
 - enhancement -> changelog entry
 - regression/major -> stable
 */
-void checkBugzilla(in ref PullRequest pr, UserMessage[] msgs)
+void checkBugzilla(in ref PullRequest pr, ref UserMessage[] msgs, in Issue[] bugzillaIssues)
 {
+    // check for stable
+    if (pr.base.ref_ != "stable")
+    {
+        if (bugzillaIssues.any!(i => i.status.among("NEW", "ASSIGNED") &&
+                                     i.severity.among("critical", "major",
+                                                      "blocker", "regression")))
+        {
+            msgs ~= UserMessage(UserMessage.Type.Warning,
+                "Regression fixes should always target stable");
+        }
+    }
 }
 
 
-UserMessage[] checkForWarnings(in PullRequest pr)
+UserMessage[] checkForWarnings(in PullRequest pr, in Issue[] bugzillaIssues)
 {
     UserMessage[] msgs;
     pr.checkDiff(msgs);
-    pr.checkBugzilla(msgs);
+    pr.checkBugzilla(msgs, bugzillaIssues);
     return msgs;
 }
 
 void printMessages(W)(W app, UserMessage[] msgs)
 {
     foreach (msg; msgs)
     {
-        app ~= " - ";
+        app ~= "- ";
         app ~= msg.text;
         app ~= "\n";
     }

test/comments.d

@@ -113,3 +113,45 @@ unittest
 
     postGitHubHook("dlang_phobos_merged_4963.json");
 }
+
+// critical bug fix (not in stable) -> show warning to target stable
+unittest
+{
+    setAPIExpectations(
+        "/github/repos/dlang/phobos/pulls/4921/commits",
+        "/github/repos/dlang/phobos/issues/4921/labels",
+        "/github/repos/dlang/phobos/issues/4921/comments", (ref Json j) {
+            j = Json.emptyArray;
+        },
+        "/bugzilla/buglist.cgi?bug_id=8573&ctype=csv&columnlist=short_desc,bug_status,resolution,bug_severity,priority",
+        (scope HTTPServerRequest req, scope HTTPServerResponse res){
+            res.writeBody(
+`bug_id,"short_desc","bug_status","resolution","bug_severity","priority"
+8573,"A simpler Phobos function that returns the index of the mix or max item","NEW","---","regression","P2"`);
+        },
+        // no bug fix label, since Issues are only referenced but not fixed according to commit messages
+        "/github/repos/dlang/phobos/issues/4921/comments",
+        (scope HTTPServerRequest req, scope HTTPServerResponse res){
+            import std.stdio;
+            assert(req.method == HTTPMethod.POST);
+            writeln(req.json["body"]);
+            auto expectedComment =
+`### Bugzilla references
+
+Fix | Bugzilla | Description
+--- | --- | ---
+✗ | [8573](%s/show_bug.cgi?id=8573) | A simpler Phobos function that returns the index of the mix or max item
+
+### Warnings
+
+- Regression fixes should always target stable
+`.format(bugzillaURL);
+            writeln(expectedComment);
+            assert(req.json["body"].get!string == expectedComment);
+            res.writeVoidBody;
+        },
+        "/trello/1/search?query=name:%22Issue%208573%22&"~trelloAuth,
+    );
+
+    postGitHubHook("dlang_phobos_synchronize_4921.json");
+}