Warn when fixing enhancements without changelog

source/dlangbot/app.d

@@ -7,7 +7,7 @@ import dlangbot.trello;
 import dlangbot.utils;
 
 public import dlangbot.bugzilla : bugzillaLogin, bugzillaPassword, bugzillaURL;
-public import dlangbot.github_api   : githubAPIURL, githubAuth, githubHookSecret;
+public import dlangbot.github_api   : githubURL, githubAPIURL, githubAuth, githubHookSecret;
 public import dlangbot.trello   : trelloAPIURL, trelloAuth, trelloSecret;
 public import dlangbot.twitter : oAuth, tweet, twitterURL, twitterEnabled;
 public import dlangbot.buildkite : buildkiteAPIURL, buildkiteAuth, buildkiteHookSecret, dlangbotAgentAuth;

source/dlangbot/github.d

@@ -52,6 +52,13 @@ string markdownEscape(string desc)
     return app.data;
 }
 
+string manualChangelogURL(string repoSlug)
+{
+    return "https://github.com/%s/blob/master/%s".format(
+        repoSlug, repoSlug == "dlang/dlang.org" ? "language-changelog" : "changelog",
+    );
+}
+
 string formatComment(in ref PullRequest pr, in IssueRef[] refs, in Issue[] descs, in UserMessage[] msgs)
 {
     import std.array : appender;
@@ -101,8 +108,8 @@ If you have addressed all reviews or aren't sure how to proceed, don't hesitate
             app.formattedWrite(
 `Your PR doesn't reference any Bugzilla issue.
 
-If your PR contains non-trivial changes, please [reference a Bugzilla issue](https://github.com/dlang-bots/dlang-bot#automated-references) or create a [manual changelog](https://github.com/%s/blob/master/%s).
-`, pr.repoSlug, pr.repoSlug == "dlang/dlang.org" ? "language-changelog" : "changelog");
+If your PR contains non-trivial changes, please [reference a Bugzilla issue](https://github.com/dlang-bots/dlang-bot#automated-references) or create a [manual changelog](%s).
+`, manualChangelogURL(pr.repoSlug));
     }
 
     if (msgs.length)

source/dlangbot/github_api.d

@@ -1,5 +1,6 @@
 module dlangbot.github_api;
 
+shared string githubURL = "https://github.com";
 shared string githubAPIURL = "https://api.github.com";
 shared string githubAuth, githubHookSecret;
 
@@ -241,6 +242,7 @@ struct PullRequest
     @name("created_at") SysTime createdAt;
     @name("updated_at") SysTime updatedAt;
     @name("closed_at") Nullable!SysTime closedAt;
+    @name("diff_url") string diffURL;
     bool locked;
     // TODO: update payloads
     //@name("maintainer_can_modify") bool maintainerCanModify;

source/dlangbot/warnings.d

@@ -1,7 +1,7 @@
 module dlangbot.warnings;
 
 import dlangbot.bugzilla : Issue, IssueRef;
-import dlangbot.github : PullRequest;
+import dlangbot.github : PullRequest, manualChangelogURL;
 
 import std.algorithm;
 
@@ -14,9 +14,26 @@ struct UserMessage
 }
 
 
-// check diff length
-void checkDiff(in ref PullRequest pr, ref UserMessage[] msgs)
+void checkEnhancementChangelog(in ref PullRequest pr, ref UserMessage[] msgs,
+        in Issue[] bugzillaIssues, in IssueRef[] refs)
 {
+    import dlangbot.utils : request, expectOK;
+    import vibe.stream.operations : readAllUTF8;
+    import std.format : format;
+
+    if (bugzillaIssues.any!(i => i.status.among("NEW", "ASSIGNED", "REOPENED") &&
+                                 i.severity == "enhancement" &&
+                                 refs.canFind!(r => r.id == i.id && r.fixed)))
+    {
+        auto diff = request(pr.diffURL).expectOK.bodyReader.readAllUTF8;
+        if (!diff.canFind("\n+++ b/changelog/"))
+        {
+            msgs ~= UserMessage(UserMessage.Type.Warning,
+                "Pull requests implementing enhancements should include a [full changelog entry](%s)."
+                .format(manualChangelogURL(pr.repoSlug))
+            );
+        }
+    }
 }
 
 
@@ -55,8 +72,8 @@ git rebase --onto upstream/stable upstream/master
 UserMessage[] checkForWarnings(in PullRequest pr, in Issue[] bugzillaIssues, in IssueRef[] refs)
 {
     UserMessage[] msgs;
-    pr.checkDiff(msgs);
     pr.checkBugzilla(msgs, bugzillaIssues, refs);
+    pr.checkEnhancementChangelog(msgs, bugzillaIssues, refs);
     return msgs;
 }
 

test/bugzilla.d

@@ -225,7 +225,7 @@ unittest
             assert("keywords" in req.json["params"][0]);
 
             auto comment = req.json["params"][0]["comment"]["body"].get!string;
-            enum expected = q"EOF
+            auto expected = q"EOF
 @MartinNowak created dlang/dmd pull request #6359 "fix Issue 16794 - dmd not working on Ubuntu 16.10" fixing this issue:
 
 - fix Issue 16794 - dmd not working on Ubuntu 16.10
@@ -235,7 +235,7 @@ unittest
   - also see https://github.com/dlang/installer/pull/207
 
 https://github.com/dlang/dmd/pull/6359
-EOF".chomp;
+EOF".chomp.replace("https://github.com/dlang/installer", githubURL ~ "/dlang/installer"); // compensate for API reference rewriting
             assert(comment == expected, comment);
 
             auto j = Json(["error" : Json(null), "result" : Json.emptyObject]);

test/comments.d

@@ -131,6 +131,7 @@ unittest
 8573,"A simpler Phobos function that returns the index of the mix or max item","NEW","---","enhancement","P2",`);
         },
         "/github/repos/dlang/phobos/issues/4921/labels",
+        "/github.com/dlang/phobos/pull/4921.diff",
         "/github/repos/dlang/phobos/issues/4921/labels", (ref Json j) {
             j = Json.emptyArray;
         },
@@ -169,6 +170,7 @@ unittest
 8574,"Some Bug","NEW","---","normal","P2",`);
         },
         "/github/repos/dlang/phobos/issues/4921/labels",
+        "/github.com/dlang/phobos/pull/4921.diff",
         "/github/repos/dlang/phobos/issues/4921/labels", (ref Json j) {
             j = Json.emptyArray;
         },
@@ -200,6 +202,7 @@ unittest
          "/github/repos/dlang/phobos/issues/4921/comments",
         "/bugzilla/buglist.cgi?bug_id=8573&ctype=csv&columnlist=short_desc,bug_status,resolution,bug_severity,priority,keywords",
         "/github/repos/dlang/phobos/issues/4921/labels",
+        "/github.com/dlang/phobos/pull/4921.diff",
         "/github/repos/dlang/phobos/issues/4921/labels", (ref Json j) {
             j[0]["name"] = "Enhancement";
         },
@@ -558,3 +561,89 @@ unittest
     assert("a\\b".markdownEscape == "a\\\\b");
     assert("a+b".markdownEscape == r"a\+b");
 }
+
+@("enhancement-with-changelog")
+unittest
+{
+    setAPIExpectations(
+        "/github/repos/dlang/phobos/pulls/4921/commits", (ref Json j) {
+            j[0]["commit"]["message"] = "Fix Issue 8573";
+        },
+         "/github/repos/dlang/phobos/issues/4921/comments",
+        "/bugzilla/buglist.cgi?bug_id=8573&ctype=csv&columnlist=short_desc,bug_status,resolution,bug_severity,priority,keywords",
+        (scope HTTPServerRequest req, scope HTTPServerResponse res){
+            res.writeBody(
+`bug_id,"short_desc","bug_status","resolution","bug_severity","priority","keywords"
+8573,"A simpler Phobos function that returns the index of the mix or max item","NEW","---","enhancement","P2",`);
+        },
+        "/github/repos/dlang/phobos/issues/4921/labels",
+        "/github.com/dlang/phobos/pull/4921.diff", (scope HTTPServerRequest req, scope HTTPServerResponse res){
+            res.writeBody(q"EOF
+diff --git a/changelog/bla-bla.dd b/changelog/bla-bla.dd
+new file mode 100644
+index 00000000000..0147f501f08
+--- /dev/null
++++ b/changelog/bla-bla.dd
+@@ -0,0 +1,1 @@
++bla bla
+EOF");
+        },
+        "/github/repos/dlang/phobos/issues/4921/labels", (ref Json j) {
+            j = Json.emptyArray;
+        },
+        "/github/orgs/dlang/public_members?per_page=100",
+        "/github/repos/dlang/phobos/issues/comments/262784442",
+        (scope HTTPServerRequest req, scope HTTPServerResponse res){
+            assert(req.method == HTTPMethod.PATCH);
+            assert(!req.json["body"].get!string.canFind("Pull requests implementing enhancements should include"));
+            res.writeVoidBody;
+        },
+        "/github/repos/dlang/phobos/issues/4921/labels",
+        (scope HTTPServerRequest req, scope HTTPServerResponse res){
+            assert(req.json[].equal(["Enhancement"]));
+        },
+        "/trello/1/search?query=name:%22Issue%208573%22&"~trelloAuth,
+        "/bugzilla/jsonrpc.cgi", // Bug.comments
+        "/bugzilla/jsonrpc.cgi", // Bug.update
+    );
+
+    postGitHubHook("dlang_phobos_synchronize_4921.json");
+}
+
+@("enhancement-with-changelog")
+unittest
+{
+    setAPIExpectations(
+        "/github/repos/dlang/phobos/pulls/4921/commits", (ref Json j) {
+            j[0]["commit"]["message"] = "Fix Issue 8573";
+        },
+         "/github/repos/dlang/phobos/issues/4921/comments",
+        "/bugzilla/buglist.cgi?bug_id=8573&ctype=csv&columnlist=short_desc,bug_status,resolution,bug_severity,priority,keywords",
+        (scope HTTPServerRequest req, scope HTTPServerResponse res){
+            res.writeBody(
+`bug_id,"short_desc","bug_status","resolution","bug_severity","priority","keywords"
+8573,"A simpler Phobos function that returns the index of the mix or max item","NEW","---","enhancement","P2",`);
+        },
+        "/github/repos/dlang/phobos/issues/4921/labels",
+        "/github.com/dlang/phobos/pull/4921.diff",
+        "/github/repos/dlang/phobos/issues/4921/labels", (ref Json j) {
+            j = Json.emptyArray;
+        },
+        "/github/orgs/dlang/public_members?per_page=100",
+        "/github/repos/dlang/phobos/issues/comments/262784442",
+        (scope HTTPServerRequest req, scope HTTPServerResponse res){
+            assert(req.method == HTTPMethod.PATCH);
+            assert(req.json["body"].get!string.canFind("Pull requests implementing enhancements should include"));
+            res.writeVoidBody;
+        },
+        "/github/repos/dlang/phobos/issues/4921/labels",
+        (scope HTTPServerRequest req, scope HTTPServerResponse res){
+            assert(req.json[].equal(["Enhancement"]));
+        },
+        "/trello/1/search?query=name:%22Issue%208573%22&"~trelloAuth,
+        "/bugzilla/jsonrpc.cgi", // Bug.comments
+        "/bugzilla/jsonrpc.cgi", // Bug.update
+    );
+
+    postGitHubHook("dlang_phobos_synchronize_4921.json");
+}

test/utils.d

@@ -83,6 +83,7 @@ void startFakeAPIServer()
     auto fakeAPIServerURL = "http://" ~ fakeSettings.bindAddresses[0] ~ ":"
                                       ~ fakeSettings.port.to!string;
 
+    githubURL = fakeAPIServerURL ~ "/github.com";
     githubAPIURL = fakeAPIServerURL ~ "/github";
     trelloAPIURL = fakeAPIServerURL ~ "/trello";
     buildkiteAPIURL = fakeAPIServerURL ~ "/buildkite";
@@ -156,9 +157,10 @@ auto payloadServer(scope HTTPServerRequest req, scope HTTPServerResponse res)
     {
         logInfo("reading payload: %s", filePath);
         auto payload = filePath.readText;
-        if (req.requestURL.startsWith("/github", "/trello", "/buildkite", "/hcloud"))
+        if (req.requestURL.startsWith("/github/", "/trello/", "/buildkite/", "/hcloud/"))
         {
             auto payloadJson = payload.parseJsonString;
+            replaceAPIReferences("https://github.com", githubURL, payloadJson);
             replaceAPIReferences("https://api.github.com", githubAPIURL, payloadJson);
             replaceAPIReferences("https://api.trello.com", trelloAPIURL, payloadJson);
             replaceAPIReferences("https://api.buildkite.com/v2", buildkiteAPIURL, payloadJson);
@@ -175,6 +177,11 @@ auto payloadServer(scope HTTPServerRequest req, scope HTTPServerResponse res)
     }
 }
 
+void replaceAPIReferences(string official, string local, ref string str)
+{
+    str = str.replace(official, local);
+}
+
 void replaceAPIReferences(string official, string local, ref Json json)
 {
     void recursiveReplace(ref Json j)
@@ -188,7 +195,10 @@ void replaceAPIReferences(string official, string local, ref Json json)
         case Json.Type.string:
             string v = j.get!string;
             if (v.countUntil(official) >= 0)
-                j = v.replace(official, local);
+            {
+                replaceAPIReferences(official, local, v);
+                j = v;
+            }
             break;
         default:
             break;
@@ -294,19 +304,23 @@ void postGitHubHook(string payload, string eventType = "pull_request",
     auto req = requestHTTP(ghTestHookURL, (scope req) {
         req.method = HTTPMethod.POST;
 
-        auto payload = payload.readText.parseJsonString;
+        auto payload = payload.readText;
 
         // localize accessed URLs
+        replaceAPIReferences("https://github.com", githubURL, payload);
         replaceAPIReferences("https://api.github.com", githubAPIURL, payload);
 
         req.headers["X-GitHub-Event"] = eventType;
 
         if (postprocess !is null)
-            postprocess(payload, req);
+        {
+            auto payloadJson = payload.parseJsonString;
+            postprocess(payloadJson, req);
+            payload = payloadJson.toString;
+        }
 
-        auto respStr = payload.toString;
-        req.headers["X-Hub-Signature"] = getSignature(respStr);
-        req.writeBody(cast(ubyte[]) respStr);
+        req.headers["X-Hub-Signature"] = getSignature(payload);
+        req.writeBody(cast(ubyte[]) payload);
     });
     assert(req.statusCode == 200,
         "Request failed with status %d. Response body:\n\n%s"