diff --git a/data/payloads/bugzilla_buglist.cgi?bug_id=16794&ctype=csv&columnlist=short_desc b/data/payloads/bugzilla_buglist.cgi?bug_id=16794&ctype=csv&columnlist=short_desc deleted file mode 100644 index 63e648f..0000000 --- a/data/payloads/bugzilla_buglist.cgi?bug_id=16794&ctype=csv&columnlist=short_desc +++ /dev/null @@ -1,2 +0,0 @@ -bug_id,"short_desc" -16794,"dmd not working on Ubuntu 16.10 because of default PIE linking" \ No newline at end of file diff --git a/data/payloads/bugzilla_buglist.cgi?bug_id=16794&ctype=csv&columnlist=short_desc,bug_status,resolution,bug_severity,priority b/data/payloads/bugzilla_buglist.cgi?bug_id=16794&ctype=csv&columnlist=short_desc,bug_status,resolution,bug_severity,priority new file mode 100644 index 0000000..70b504d --- /dev/null +++ b/data/payloads/bugzilla_buglist.cgi?bug_id=16794&ctype=csv&columnlist=short_desc,bug_status,resolution,bug_severity,priority @@ -0,0 +1,2 @@ +bug_id,"short_desc","bug_status","resolution","bug_severity","priority" +16794,"dmd not working on Ubuntu 16.10 because of default PIE linking","RESOLVED","FIXED","critical","P1" \ No newline at end of file diff --git a/data/payloads/bugzilla_buglist.cgi?bug_id=8573&ctype=csv&columnlist=short_desc b/data/payloads/bugzilla_buglist.cgi?bug_id=8573&ctype=csv&columnlist=short_desc deleted file mode 100644 index 09fe367..0000000 --- a/data/payloads/bugzilla_buglist.cgi?bug_id=8573&ctype=csv&columnlist=short_desc +++ /dev/null @@ -1,2 +0,0 @@ -bug_id,"short_desc" -8573,"A simpler Phobos function that returns the index of the mix or max item" \ No newline at end of file diff --git a/data/payloads/bugzilla_buglist.cgi?bug_id=8573&ctype=csv&columnlist=short_desc,bug_status,resolution,bug_severity,priority b/data/payloads/bugzilla_buglist.cgi?bug_id=8573&ctype=csv&columnlist=short_desc,bug_status,resolution,bug_severity,priority new file mode 100644 index 0000000..425073f --- /dev/null +++ b/data/payloads/bugzilla_buglist.cgi?bug_id=8573&ctype=csv&columnlist=short_desc,bug_status,resolution,bug_severity,priority @@ -0,0 +1,2 @@ +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","---","enhancement","P2" \ No newline at end of file diff --git a/source/dlangbot/app.d b/source/dlangbot/app.d index f12cb40..5d8a1c4 100644 --- a/source/dlangbot/app.d +++ b/source/dlangbot/app.d @@ -140,6 +140,7 @@ void handlePR(string action, PullRequest* _pr) { import std.algorithm : any; import vibe.core.core : setTimer; + import dlangbot.warnings : checkForWarnings, UserMessage; const PullRequest pr = *_pr; @@ -172,7 +173,13 @@ void handlePR(string action, PullRequest* _pr) auto descs = getDescriptions(refs); auto comment = pr.getBotComment; - pr.updateGithubComment(comment, action, refs, descs); + UserMessage[] msgs; + if (action == "opened" || action == "synchronize") + { + msgs = pr.checkForWarnings(descs); + } + + pr.updateGithubComment(comment, action, refs, descs, msgs); if (refs.any!(r => r.fixed) && comment.body_.length == 0) pr.addLabels(["Bug fix"]); diff --git a/source/dlangbot/bugzilla.d b/source/dlangbot/bugzilla.d index 3564d5f..7d470f4 100644 --- a/source/dlangbot/bugzilla.d +++ b/source/dlangbot/bugzilla.d @@ -47,7 +47,15 @@ IssueRef[] getIssueRefs(Json[] commits) return issues; } -struct Issue { int id; string desc; } +struct Issue +{ + int id; + string desc; + string status; + string resolution; + string severity; + string priority; +} // get pairs of (issue number, short descriptions) from bugzilla Issue[] getDescriptions(R)(R issueRefs) @@ -58,7 +66,7 @@ Issue[] getDescriptions(R)(R issueRefs) if (issueRefs.empty) return null; - return "%s/buglist.cgi?bug_id=%(%d,%)&ctype=csv&columnlist=short_desc" + return "%s/buglist.cgi?bug_id=%(%d,%)&ctype=csv&columnlist=short_desc,bug_status,resolution,bug_severity,priority" .format(bugzillaURL, issueRefs.map!(r => r.id)) .requestHTTP .bodyReader.readAllUTF8 @@ -67,5 +75,3 @@ Issue[] getDescriptions(R)(R issueRefs) .sort!((a, b) => a.id < b.id) .release; } - - diff --git a/source/dlangbot/github.d b/source/dlangbot/github.d index 8d589ad..74169b0 100644 --- a/source/dlangbot/github.d +++ b/source/dlangbot/github.d @@ -4,10 +4,11 @@ string githubAPIURL = "https://api.github.com"; string githubAuth, hookSecret; import dlangbot.bugzilla : bugzillaURL, Issue, IssueRef; +import dlangbot.warnings : printMessages, UserMessage; import std.algorithm, std.range; import std.datetime; -import std.format : format; +import std.format : format, formattedWrite; import std.typecons : Tuple; import vibe.core.log; @@ -20,12 +21,25 @@ import vibe.stream.operations : readAllUTF8; // Github comments //============================================================================== -string formatComment(in ref PullRequest pr, in IssueRef[] refs, in Issue[] descs) +void printBugList(W)(W app, in IssueRef[] refs, in Issue[] descs) +{ + auto combined = zip(refs.map!(r => r.id), refs.map!(r => r.fixed), descs.map!(d => d.desc)); + app.put("Fix | Bugzilla | Description\n"); + app.put("--- | --- | ---\n"); + foreach (num, closed, desc; combined) + { + app.formattedWrite( + "%1$s | [%2$s](%4$s/show_bug.cgi?id=%2$s) | %3$s\n", + closed ? "✓" : "✗", num, desc, bugzillaURL); + } +} + +string formatComment(in ref PullRequest pr, in IssueRef[] refs, in Issue[] descs, in UserMessage[] msgs) { import std.array : appender; - import std.format : formattedWrite; - auto app = appender!string(); + auto app = appender!string; + app.formattedWrite( `Thanks for your pull request, @%s! We are looking forward to reviewing it, and you should be hearing from a maintainer soon. @@ -43,17 +57,17 @@ Please see [CONTRIBUTING.md](https://github.com/%s/blob/master/CONTRIBUTING.md) `, pr.user.login, pr.repoSlug); - if (refs.length > 0) + if (refs.length) { - auto combined = zip(refs.map!(r => r.id), refs.map!(r => r.fixed), descs.map!(d => d.desc)); - app.put("Fix | Bugzilla | Description\n"); - app.put("--- | --- | ---\n"); - foreach (num, closed, desc; combined) - { - app.formattedWrite( - "%1$s | [%2$s](%4$s/show_bug.cgi?id=%2$s) | %3$s\n", - closed ? "✓" : "✗", num, desc, bugzillaURL); - } + app ~= "### Bugzilla references\n\n"; + app.printBugList(refs, descs); + } + if (msgs.length) + { + if (refs.length) + app ~= "\n"; + app ~= "### Warnings\n\n"; + app.printMessages(msgs); } return app.data; } @@ -113,13 +127,14 @@ auto ghSendRequest(T...)(HTTPMethod method, string url, T arg) }, url); } -void updateGithubComment(in ref PullRequest pr, in ref GHComment comment, string action, IssueRef[] refs, Issue[] descs) +void updateGithubComment(in ref PullRequest pr, in ref GHComment comment, + string action, IssueRef[] refs, Issue[] descs, UserMessage[] msgs) { logDebug("%s", refs); logDebug("%s", descs); assert(refs.map!(r => r.id).equal(descs.map!(d => d.id))); - auto msg = pr.formatComment(refs, descs); + auto msg = pr.formatComment(refs, descs, msgs); logDebug("%s", msg); if (msg != comment.body_) @@ -437,6 +452,7 @@ struct PullRequest static struct Branch { string sha; + string ref_; Repo repo; } Branch base, head; diff --git a/source/dlangbot/warnings.d b/source/dlangbot/warnings.d new file mode 100644 index 0000000..086fe09 --- /dev/null +++ b/source/dlangbot/warnings.d @@ -0,0 +1,60 @@ +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; +} + + +// check diff length +void checkDiff(in ref PullRequest pr, ref UserMessage[] msgs) +{ +} + + +/** +Check bugzilla priority +- enhancement -> changelog entry +- regression/major -> stable +*/ +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, in Issue[] bugzillaIssues) +{ + UserMessage[] msgs; + pr.checkDiff(msgs); + pr.checkBugzilla(msgs, bugzillaIssues); + return msgs; +} + +void printMessages(W)(W app, in UserMessage[] msgs) +{ + foreach (msg; msgs) + { + app ~= "- "; + app ~= msg.text; + app ~= "\n"; + } +} diff --git a/test/comments.d b/test/comments.d index 0776fc8..8645b70 100644 --- a/test/comments.d +++ b/test/comments.d @@ -9,12 +9,14 @@ unittest "/github/repos/dlang/phobos/pulls/4921/commits", "/github/repos/dlang/phobos/issues/4921/labels", "/github/repos/dlang/phobos/issues/4921/comments", - "/bugzilla/buglist.cgi?bug_id=8573&ctype=csv&columnlist=short_desc", + "/bugzilla/buglist.cgi?bug_id=8573&ctype=csv&columnlist=short_desc,bug_status,resolution,bug_severity,priority", "/github/repos/dlang/phobos/issues/comments/262784442", (scope HTTPServerRequest req, scope HTTPServerResponse res){ assert(req.method == HTTPMethod.PATCH); auto expectedComment = -`Fix | Bugzilla | Description +`### 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 `.format(bugzillaURL); @@ -35,13 +37,15 @@ unittest "/github/repos/dlang/phobos/issues/4921/comments", (ref Json j) { j = Json.emptyArray; }, - "/bugzilla/buglist.cgi?bug_id=8573&ctype=csv&columnlist=short_desc", + "/bugzilla/buglist.cgi?bug_id=8573&ctype=csv&columnlist=short_desc,bug_status,resolution,bug_severity,priority", // 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){ assert(req.method == HTTPMethod.POST); auto expectedComment = -`Fix | Bugzilla | Description +`### 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 `.format(bugzillaURL); @@ -140,3 +144,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.canFind(expectedComment)); + res.writeVoidBody; + }, + "/trello/1/search?query=name:%22Issue%208573%22&"~trelloAuth, + ); + + postGitHubHook("dlang_phobos_synchronize_4921.json"); +} diff --git a/test/trello.d b/test/trello.d index 7db8d40..4384f9c 100644 --- a/test/trello.d +++ b/test/trello.d @@ -6,7 +6,7 @@ import std.stdio, std.format : format; unittest { setAPIExpectations( - "/bugzilla/buglist.cgi?bug_id=16794&ctype=csv&columnlist=short_desc", + "/bugzilla/buglist.cgi?bug_id=16794&ctype=csv&columnlist=short_desc,bug_status,resolution,bug_severity,priority", "/trello/1/cards/583f517a333add7c28e0cec7/actions?filter=commentCard&"~trelloAuth, (ref Json j) { j = Json.emptyArray; }, "/trello/1/cards/583f517a333add7c28e0cec7/actions/comments?"~trelloAuth, @@ -25,7 +25,7 @@ unittest unittest { setAPIExpectations( - "/bugzilla/buglist.cgi?bug_id=16794&ctype=csv&columnlist=short_desc", + "/bugzilla/buglist.cgi?bug_id=16794&ctype=csv&columnlist=short_desc,bug_status,resolution,bug_severity,priority", "/trello/1/cards/583f517a333add7c28e0cec7/actions?filter=commentCard&"~trelloAuth, (ref Json j) { j[0]["data"]["text"] = "- [Issue 16794 - bla bla](https://issues.dlang.org/show_bug.cgi?id=16794)\n"; }, "/trello/1/cards/583f517a333add7c28e0cec7/actions/583f517b91413ef81f1f9d34/comments?"~trelloAuth, @@ -46,7 +46,7 @@ unittest setAPIExpectations( "/github/repos/dlang/dmd/pulls/6359/commits", "/github/repos/dlang/dmd/issues/6359/comments", - "/bugzilla/buglist.cgi?bug_id=16794&ctype=csv&columnlist=short_desc", + "/bugzilla/buglist.cgi?bug_id=16794&ctype=csv&columnlist=short_desc,bug_status,resolution,bug_severity,priority", "/github/repos/dlang/dmd/issues/6359/comments", // action: add bug fix label "/github/repos/dlang/dmd/issues/6359/labels", @@ -79,7 +79,7 @@ unittest setAPIExpectations( "/github/repos/dlang/dmd/pulls/6359/commits", "/github/repos/dlang/dmd/issues/6359/comments", - "/bugzilla/buglist.cgi?bug_id=16794&ctype=csv&columnlist=short_desc", + "/bugzilla/buglist.cgi?bug_id=16794&ctype=csv&columnlist=short_desc,bug_status,resolution,bug_severity,priority", "/github/repos/dlang/dmd/issues/6359/comments", // action: add bug fix label "/github/repos/dlang/dmd/issues/6359/labels",