Merge pull request #302 from CyberShadow/pull-20230624-050612

First phase of GitHub Issues support

README.md

@@ -22,12 +22,12 @@ Features
 Automated references
 --------------------
 
-For example let's say you fixed [Issue 16582](https://issues.dlang.org/show_bug.cgi?id=16582)
+For example let's say you fixed [Bugzilla Issue 16582](https://issues.dlang.org/show_bug.cgi?id=16582)
 and make a PR for on GitHub.
 If one of your commits mentions the issue, e.g. like this Git commit message:
 
 ```
-fix Issue 16582 - ParameterDefaults fails w/ scope parameter
+fix Bugzilla Issue 16582 - ParameterDefaults fails w/ scope parameter
 ```
 
 The Dlang-Bot will do all the hard work of linking and referencing
@@ -68,7 +68,7 @@ Using this syntax is also very important because for the changelog generation, t
 git history will be used. Thus _only_ if the Dlang-Bot has detected an issue
 and commented on your PR it can become part of the changelog.
 
-In doubt, you can use e.g. [Regex101](https://regex101.com/r/aI0Rp6/7) to validate your commit message.
+In doubt, you can use e.g. [Regex101](https://regex101.com/r/vjOAM4/1) to validate your commit message.
 
 ### Referencing multiple issues
 

data/hooks/trello/active_issue_16794.json

@@ -58,7 +58,7 @@
  "card": {
  "shortLink": "H1IPmxHc",
  "idShort": 279,
- "name": "Test Card Issue 16794",
+ "name": "Test Card Bugzilla 16794",
  "id": "583f517a333add7c28e0cec7"
  }
  },

data/payloads/github_repos_dlang_dmd_pulls_6359_commits

@@ -12,7 +12,7 @@
  "email": "[email protected]",
  "date": "2016-12-24T17:58:32Z"
  },
- "message": "fix Issue 16794 - dmd not working on Ubuntu 16.10\n\n- enable PIC by default on amd64 linux (no significant overhead, full\n PIC/PIE support)\n- also see https://github.com/dlang/installer/pull/207",
+ "message": "fix Bugzilla Issue 16794 - dmd not working on Ubuntu 16.10\n\n- enable PIC by default on amd64 linux (no significant overhead, full\n PIC/PIE support)\n- also see https://github.com/dlang/installer/pull/207",
  "tree": {
  "sha": "7fe104c3fc612cb2b527eab295f858d2e14796a3",
  "url": "https://api.github.com/repos/dlang/dmd/git/trees/7fe104c3fc612cb2b527eab295f858d2e14796a3"

data/payloads/github_repos_dlang_phobos_pulls_4921_commits

@@ -12,7 +12,7 @@
  "email": "[email protected]",
  "date": "2016-12-16T09:11:07Z"
  },
- "message": "Issue 8573 - A simpler Phobos function that returns the index of the mix or max item\n\nIssue 8573 - A simpler Phobos function that returns the index of the mix or max item\n\nadded some review fixes\n\nfixed an issue with a mutable variable\n\nApplied review feedback\n\nRenamed functions to minIndex and maxIndex + used sizediff_t for return value type\n\nUpdated function so that it works optimally even for lazy ranges and algorithms\n\nReverted to having only copyable elements in ranges\n\nAdded more unittests; implemented an array path; fixed documentation\n\nSquashed commits",
+ "message": "Bugzilla Issue 8573 - A simpler Phobos function that returns the index of the mix or max item\n\nBugzilla Issue 8573 - A simpler Phobos function that returns the index of the mix or max item\n\nadded some review fixes\n\nfixed an issue with a mutable variable\n\nApplied review feedback\n\nRenamed functions to minIndex and maxIndex + used sizediff_t for return value type\n\nUpdated function so that it works optimally even for lazy ranges and algorithms\n\nReverted to having only copyable elements in ranges\n\nAdded more unittests; implemented an array path; fixed documentation\n\nSquashed commits",
  "tree": {
  "sha": "dae14cbc7dcd78423698c41f9f20d30b89d249c3",
  "url": "https://api.github.com/repos/dlang/phobos/git/trees/dae14cbc7dcd78423698c41f9f20d30b89d249c3"

data/payloads/github_repos_dlang_phobos_pulls_5519_commits

@@ -12,7 +12,7 @@
  "email": "[email protected]",
  "date": "2017-06-28T23:45:01Z"
  },
- "message": "Fix issue 17564: Eliminate \"static this\" for theAllocator\n\nThis switches to lazy initialization of theAllocator, so that accessing it form within `shared static this` works as expected.",
+ "message": "Fix bugzilla issue 17564: Eliminate \"static this\" for theAllocator\n\nThis switches to lazy initialization of theAllocator, so that accessing it form within `shared static this` works as expected.",
  "tree": {
  "sha": "b490839e4489d62864e0aa8aab9e308a535a8ad8",
  "url": "https://api.github.com/repos/dlang/phobos/git/trees/b490839e4489d62864e0aa8aab9e308a535a8ad8"

source/dlangbot/app.d

@@ -248,9 +248,8 @@ void handlePR(string action, PullRequest* _pr)
  refs = getIssueRefs(commits);
  descs = getDescriptions(refs);
  if (action == "opened" || action == "synchronize" || action == "ready_for_review")
- {
- msgs = pr.checkForWarnings(descs, refs);
- }
+ msgs ~= pr.checkForWarnings(descs, refs);
+ msgs ~= checkLegacyIssueRefs(commits);
 
  pr.updateGithubComment(comment, action, refs, descs, msgs);
 

source/dlangbot/bugzilla.d

@@ -13,15 +13,23 @@ static immutable bugzillaProjectSlugs = ["dlang/dmd", "dlang/druntime", "dlang/p
 import std.algorithm, std.conv, std.range, std.string;
 import std.exception : enforce;
 import std.format : format;
+import std.regex : ctRegex, matchFirst, matchAll;
+
+import dlangbot.warnings : UserMessage;
 
 //==============================================================================
 // Bugzilla
 //==============================================================================
 
-auto matchIssueRefs(string message)
-{
- import std.regex;
+private enum issueRE = ctRegex!(
+ `(?:^fix(?:es)?(?:\s+bugzilla)(?:\s+(?:issues?|bugs?))?\s+(#?\d+(?:[\s,\+&and]+#?\d+)*))|` ~
+ `(?:bugzilla\s+(?:(?:issues?|bugs?)\s+)?(#?\d+(?:[\s,\+&and]+#?\d+)*))`, "im");
+private enum oldIssueRE = ctRegex!(
+ `(?:^fix(?:es)?(?:\s+(?:issues?|bugs?))?\s+(#?\d+(?:[\s,\+&and]+#?\d+)*))|` ~
+ `(?:(?:issues?|bugs?)\s+(#?\d+(?:[\s,\+&and]+#?\d+)*))`, "im");
 
+auto matchIssueRefs(RE)(string message, RE re = issueRE)
+{
  static auto matchToRefs(M)(M m)
  {
  enum splitRE = ctRegex!(`(\d+)`);
@@ -31,44 +39,44 @@ auto matchIssueRefs(string message)
  .map!(match => IssueRef(match.hit.to!int, closed));
  }
 
- enum issueRE = ctRegex!(`(?:^fix(?:es)?(?:\s+(?:issues?|bugs?))?\s+(#?\d+(?:[\s,\+&and]+#?\d+)*))|` ~
- `(?:(?:issues?|bugs?)\s+(#?\d+(?:[\s,\+&and]+#?\d+)*))`, "im");
- return matchToRefs(message.matchFirst(issueRE));
+ return matchToRefs(message.matchFirst(re));
 }
 
 unittest
 {
- assert(equal(matchIssueRefs("fix issue 16319 and fix std.traits.isInnerClass"),
+ assert(equal(matchIssueRefs("fix bugzilla issue 16319 and fix std.traits.isInnerClass"),
  [IssueRef(16319, true)]));
- assert(equal(matchIssueRefs("Fixes issues 17494, 17505, 17506"),
+ assert(equal(matchIssueRefs("Fixes Bugzilla issues 17494, 17505, 17506"),
  [IssueRef(17494, true),IssueRef(17505, true), IssueRef(17506, true)]));
- assert(equal(matchIssueRefs("Fix issues 42, 55, 98: Baguette poisson fraise"),
+ assert(equal(matchIssueRefs("Fix bugzilla issues 42, 55, 98: Baguette poisson fraise"),
  [ IssueRef(42, true), IssueRef(55, true), IssueRef(98, true) ]));
  // Multi-line
- assert(equal(matchIssueRefs("Bla bla bla\n\nFixes issue #123"),
+ assert(equal(matchIssueRefs("Bla bla bla\n\nFixes bugzilla issue #123"),
  [IssueRef(123, true)]));
  // only first match considered, see #175
- assert(equal(matchIssueRefs("Fixes Issues 1234 and 2345\nblabla\nFixes Issue 3456"),
+ assert(equal(matchIssueRefs("Fixes BugZilla Issues 1234 and 2345\nblabla\nFixes BugZilla Issue 3456"),
  [IssueRef(1234, true), IssueRef(2345, true)]));
  // Related, but not closing
- assert(equal(matchIssueRefs("Issue 242: Refactor prior to fix"),
+ assert(equal(matchIssueRefs("Bugzilla Issue 242: Refactor prior to fix"),
  [IssueRef(242, false)]));
- assert(equal(matchIssueRefs("Bug 123: Add a test"),
+ assert(equal(matchIssueRefs("Bugzilla Bug 123: Add a test"),
  [IssueRef(123, false)]));
- assert(equal(matchIssueRefs("Issue #456: Improve error message"),
+ assert(equal(matchIssueRefs("Bugzilla Issue #456: Improve error message"),
  [IssueRef(456, false)]));
 
  // Short hand syntax
- assert(equal(matchIssueRefs("Fix 222, 333 and 42000: Baguette poisson fraise"),
+ assert(equal(matchIssueRefs("Fix Bugzilla 222, 333 and 42000: Baguette poisson fraise"),
  [ IssueRef(222, true), IssueRef(333, true), IssueRef(42000, true) ]));
- assert(equal(matchIssueRefs("Fix 4242 & 131 Baguette poisson fraise"),
+ assert(equal(matchIssueRefs("Fix Bugzilla 4242 & 131 Baguette poisson fraise"),
  [ IssueRef(4242, true), IssueRef(131, true) ]));
  // Just a reference, not a fix
- assert(equal(matchIssueRefs("Issue 242: Warn about buggy behavior"),
+ assert(equal(matchIssueRefs("Bugzilla Issue 242: Warn about buggy behavior"),
  [IssueRef(242, false)]));
- assert(equal(matchIssueRefs("Do not quite fix issue 242 but it's a start"),
+ assert(equal(matchIssueRefs("Do not quite fix bugzilla issue 242 but it's a start"),
  [IssueRef(242, false)]));
- assert(equal(matchIssueRefs("Workaround needed to make bug 131415 less deadly"),
+ assert(equal(matchIssueRefs("Workaround needed to make bugzilla bug 131415 less deadly"),
+ [IssueRef(131415, false)]));
+ assert(equal(matchIssueRefs("Workaround needed to make bugzilla 131415 less deadly"),
  [IssueRef(131415, false)]));
 
  // Shouldn't match
@@ -79,19 +87,19 @@ unittest
  assert(equal(matchIssueRefs("#4242: Reduce indentation prior to fix"), empty));
 
  // Note: This *will match* so just don't use that verb?
- assert(equal(matchIssueRefs("DMD issues 10 weird error message on shutdown"),
- [IssueRef(10, false)]));
+ // assert(equal(matchIssueRefs("DMD issues 10 weird error message on shutdown"),
+ //  [IssueRef(10, false)]));
 }
 
 struct IssueRef { int id; bool fixed; Json[] commits; }
 // get all issues mentioned in a commit
-IssueRef[] getIssueRefs(Json[] commits)
+IssueRef[] getIssueRefs(RE)(Json[] commits, RE re = issueRE)
 {
  return commits
  // Collect all issue references (range of ranges per commit)
  .map!(c => c["commit"]["message"]
  .get!string
- .matchIssueRefs
+ .matchIssueRefs(re)
  .map!((r) { r.commits = [c]; return r; })
  )
  // Join to flat list
@@ -110,8 +118,8 @@ IssueRef[] getIssueRefs(Json[] commits)
 
 unittest
 {
- Json fix(int id) { return ["commit":["message":"Fix Issue %d".format(id).Json].Json].Json; }
- Json mention(int id) { return ["commit":["message":"Issue %d".format(id).Json].Json].Json; }
+ Json fix(int id) { return ["commit":["message":"Fix Bugzilla %d".format(id).Json].Json].Json; }
+ Json mention(int id) { return ["commit":["message":"Bugzilla %d".format(id).Json].Json].Json; }
 
  assert(getIssueRefs([fix(1)]) == [IssueRef(1, true, [fix(1)])]);
  assert(getIssueRefs([mention(1)]) == [IssueRef(1, false, [mention(1)])]);
@@ -120,6 +128,32 @@ unittest
  assert(getIssueRefs([mention(1), fix(2), fix(1)]) == [IssueRef(1, true, [mention(1), fix(1)]), IssueRef(2, true, [fix(2)])]);
 }
 
+UserMessage[] checkLegacyIssueRefs(Json[] commits)
+{
+ auto oldHits = commits.getIssueRefs(oldIssueRE).map!(r => r.id);
+ auto newHits = commits.getIssueRefs(issueRE).map!(r => r.id);
+ auto onlyOld = oldHits.filter!(id => !newHits.canFind(id));
+ if (!onlyOld.empty)
+ return [UserMessage(UserMessage.Type.Warning,
+ "In preparation for migrating from Bugzilla to GitHub Issues, the issue reference syntax has changed. " ~
+ "Please add the word \"Bugzilla\" to issue references. For example, `Fix Bugzilla Issue 12345` or `Fix Bugzilla 12345`." ~
+ "(Reminder: the edit needs to be done in the Git *commit message*, not the GitHub *pull request*.)"
+ )];
+ return null;
+}
+
+unittest
+{
+ Json fixOld(int id) { return ["commit":["message":"Fix Issue %d".format(id).Json].Json].Json; }
+ Json fixNew(int id) { return ["commit":["message":"Fix Bugzilla %d".format(id).Json].Json].Json; }
+
+ assert( checkLegacyIssueRefs([]).empty);
+ assert( checkLegacyIssueRefs([fixNew(1)]).empty);
+ assert( checkLegacyIssueRefs([fixOld(1), fixNew(1)]).empty);
+ assert(!checkLegacyIssueRefs([fixOld(1)]).empty);
+ assert(!checkLegacyIssueRefs([fixOld(1), fixNew(2)]).empty);
+ }
+
 struct Issue
 {
  int id;

test/bugzilla.d

@@ -7,7 +7,7 @@ unittest
 {
  setAPIExpectations(
  "/github/repos/dlang/phobos/pulls/4963/commits", (ref Json j) {
- j[0]["commit"]["message"] = "Fix Issue 17564";
+ j[0]["commit"]["message"] = "Fix bugzilla Issue 17564";
  },
  "/github/repos/dlang/phobos/issues/4963/comments",
  "/github/repos/dlang/phobos/issues/4963/labels",
@@ -46,7 +46,7 @@ unittest
 dlang/phobos pull request #4963 "[DEMO for DIP1005] Converted imports to selective imports in std.array" was merged into master:
 
 - e064d5664f92c4b2f0866c08f6d0290ba66825ed by Andrei Alexandrescu:
- Fix Issue 17564
+ Fix bugzilla Issue 17564
 
 https://github.com/dlang/phobos/pull/4963
 EOF".chomp;
@@ -65,7 +65,7 @@ unittest
 {
  setAPIExpectations(
  "/github/repos/dlang/phobos/pulls/4963/commits", (ref Json j) {
- j[0]["commit"]["message"] = "Do something with Issue 17564";
+ j[0]["commit"]["message"] = "Do something with Bugzilla Issue 17564";
  },
  "/github/repos/dlang/phobos/issues/4963/comments",
  "/trello/1/search?query=name:%22Issue%2017564%22&"~trelloAuth,
@@ -96,7 +96,7 @@ unittest
 dlang/phobos pull request #4963 "[DEMO for DIP1005] Converted imports to selective imports in std.array" was merged into master:
 
 - e064d5664f92c4b2f0866c08f6d0290ba66825ed by Andrei Alexandrescu:
- Do something with Issue 17564
+ Do something with Bugzilla Issue 17564
 
 https://github.com/dlang/phobos/pull/4963
 EOF".chomp;
@@ -115,7 +115,7 @@ unittest
 {
  setAPIExpectations(
  "/github/repos/notdlang/bar/pulls/12347/commits", (ref Json j) {
- j[0]["commit"]["message"] = "Do something with Issue 17564";
+ j[0]["commit"]["message"] = "Do something with Bugzilla Issue 17564";
  },
  "/github/repos/notdlang/bar/issues/12347/comments",
  );
@@ -128,7 +128,7 @@ unittest
 {
  setAPIExpectations(
  "/github/repos/dlang/dub/pulls/12345/commits", (ref Json j) {
- j[0]["commit"]["message"] = "Do something with Issue 17564";
+ j[0]["commit"]["message"] = "Do something with Bugzilla Issue 17564";
  },
  "/github/repos/dlang/dub/issues/12345/comments",
  );
@@ -141,7 +141,7 @@ unittest
 {
  setAPIExpectations(
  "/github/repos/dlang/phobos/pulls/4963/commits", (ref Json j) {
- j[0]["commit"]["message"] = "Fix Issue 17564";
+ j[0]["commit"]["message"] = "Fix Bugzilla Issue 17564";
  },
  "/github/repos/dlang/phobos/issues/4963/comments",
  "/github/repos/dlang/phobos/issues/4963/labels",
@@ -161,7 +161,7 @@ unittest
 dlang/phobos pull request #4963 "[DEMO for DIP1005] Converted imports to selective imports in std.array" was merged into stable:
 
 - e064d5664f92c4b2f0866c08f6d0290ba66825ed by Andrei Alexandrescu:
- Fix Issue 17564
+ Fix Bugzilla Issue 17564
 
 https://github.com/dlang/phobos/pull/4963
 EOF".chomp;
@@ -224,7 +224,7 @@ unittest
  enum 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
+- fix Bugzilla Issue 16794 - dmd not working on Ubuntu 16.10
  
  - enable PIC by default on amd64 linux (no significant overhead, full
  PIC/PIE support)
@@ -248,7 +248,7 @@ unittest
  setAPIExpectations(
  "/github/repos/dlang/dmd/pulls/6359/commits",
  (ref Json j){
- j[0]["commit"]["message"] = "Fix Issue 20540 - (White|Black)Hole does not work with return|scope functions";
+ j[0]["commit"]["message"] = "Fix Bugzilla Issue 20540 - (White|Black)Hole does not work with return|scope functions";
  },
  "/github/repos/dlang/dmd/issues/6359/comments",
  "/bugzilla/buglist.cgi?bug_id=20540&ctype=csv&columnlist=short_desc,bug_status,resolution,bug_severity,priority,keywords",

test/comments.d

@@ -121,7 +121,7 @@ unittest
 {
  setAPIExpectations(
  "/github/repos/dlang/phobos/pulls/4921/commits", (ref Json j) {
- j[0]["commit"]["message"] = "Fix Issue 8573";
+ j[0]["commit"]["message"] = "Fix Bugzilla 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",
@@ -158,7 +158,7 @@ unittest
 {
  setAPIExpectations(
  "/github/repos/dlang/phobos/pulls/4921/commits", (ref Json j) {
- j[0]["commit"]["message"] = "Fix Issue 8573, 8574";
+ j[0]["commit"]["message"] = "Fix Bugzilla 8573, 8574";
  },
  "/github/repos/dlang/phobos/issues/4921/comments",
  "/bugzilla/buglist.cgi?bug_id=8573,8574&ctype=csv&columnlist=short_desc,bug_status,resolution,bug_severity,priority,keywords",
@@ -195,7 +195,7 @@ unittest
 {
  setAPIExpectations(
  "/github/repos/dlang/phobos/pulls/4921/commits", (ref Json j) {
- j[0]["commit"]["message"] = "Fix Issue 8573";
+ j[0]["commit"]["message"] = "Fix Bugzilla 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",
@@ -259,7 +259,7 @@ unittest
 {
  setAPIExpectations(
  "/github/repos/dlang/phobos/pulls/4921/commits", (ref Json j) {
- j[0]["commit"]["message"] = "Fix Issue 8573";
+ j[0]["commit"]["message"] = "Fix Bugzilla Issue 8573";
  },
  "/github/repos/dlang/phobos/issues/4921/labels",
  "/github/repos/dlang/phobos/issues/4921/comments", (ref Json j) {
@@ -397,7 +397,7 @@ unittest
 {
  setAPIExpectations(
  "/github/repos/dlang/phobos/pulls/4921/commits", (ref Json j) {
- j[0]["commit"]["message"] = "Issue 8573";
+ j[0]["commit"]["message"] = "Bugzilla 8573";
  },
  "/github/repos/dlang/phobos/issues/4921/labels",
  "/github/repos/dlang/phobos/issues/4921/comments", (ref Json j) {
@@ -428,7 +428,7 @@ unittest
 {
  setAPIExpectations(
  "/github/repos/dlang/phobos/pulls/4921/commits", (ref Json j) {
- j[0]["commit"]["message"] = "Fix Issue 8573";
+ j[0]["commit"]["message"] = "Fix Bugzilla 8573";
  },
  "/github/repos/dlang/phobos/issues/4921/labels",
  "/github/repos/dlang/phobos/issues/4921/comments", (ref Json j) {
@@ -495,7 +495,7 @@ unittest
 
  setAPIExpectations(
  "/github/repos/dlang/phobos/pulls/4921/commits", (ref Json j) {
- j[0]["commit"]["message"] = "Issue 19296";
+ j[0]["commit"]["message"] = "Bugzilla 19296";
  },
  "/github/repos/dlang/phobos/issues/4921/labels",
  "/github/repos/dlang/phobos/issues/4921/comments", (ref Json j) {
@@ -525,7 +525,7 @@ unittest
 
  setAPIExpectations(
  "/github/repos/dlang/phobos/pulls/4921/commits", (ref Json j) {
- j[0]["commit"]["message"] = "Issue 19296";
+ j[0]["commit"]["message"] = "Bugzilla 19296";
  },
  "/github/repos/dlang/phobos/issues/4921/labels",
  "/github/repos/dlang/phobos/issues/4921/comments", (ref Json j) {