Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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"

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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"
9 changes: 8 additions & 1 deletion source/dlangbot/app.d
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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"]);
Expand Down
14 changes: 10 additions & 4 deletions source/dlangbot/bugzilla.d
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -67,5 +75,3 @@ Issue[] getDescriptions(R)(R issueRefs)
.sort!((a, b) => a.id < b.id)
.release;
}


48 changes: 32 additions & 16 deletions source/dlangbot/github.d
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.

Expand All @@ -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;
}
Expand Down Expand Up @@ -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_)
Expand Down Expand Up @@ -437,6 +452,7 @@ struct PullRequest
static struct Branch
{
string sha;
string ref_;
Repo repo;
}
Branch base, head;
Expand Down
60 changes: 60 additions & 0 deletions source/dlangbot/warnings.d
Original file line number Diff line number Diff line change
@@ -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";
}
}
54 changes: 50 additions & 4 deletions test/comments.d
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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");
}
8 changes: 4 additions & 4 deletions test/trello.d
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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",
Expand Down Expand Up @@ -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",
Expand Down