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_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

@@ -14,9 +14,24 @@ 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;
+
+ 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](https://github.com/dlang/dmd/tree/master/changelog)."
+ );
+ }
+ }
 }
 
 
@@ -55,8 +70,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"