Point out common mistakes

[wilzbach]

Especially at Phobos it's a lot of work to point out common mistakes and I think we should automate this. An incomplete list of common mistakes

1. Bugfix without unittest
2. Bugfix without Bugzilla reference in correct syntax
3. Style / whitespace violations

• not everything is covered by make -f posix.mak style
  4) ...

Another big problem is that test failures are often hard to interpret for newcomers. Maybe we could have a simple heuristic that comments if there has been a failure and the user has submitted less than five PRs to Phobos with helpful information about how to deal with CI errors?

[wilzbach]

Bugfix without unittest

We can get the diff nicely from the GH API:

https://patch-diff.githubusercontent.com/raw/dlang/phobos/pull/4900.diff

so the logic could be as simple as:

if (pr.hasIssueRefs())
{
   auto diff = getDiff();
   if (!diff.canFind("unittest"))
      sendWarningComment();
}

However I am not sure whether this will work for DMD / Druntime and it's worth risking the potential false positives.

[MartinNowak]

We should rather simplify our CI systems than adding an interpret layer on top of this.
Otherwise an interesting idea, we should do some analysis on PRs to figure out how much of a problem this is. I'm usually fairly fine with manually telling that to first time commiters and they quickly pick up how things work. At least for dmd we're not overwhelmed by first-time contributions, so automation might not be that interesting (yet).

[wilzbach]

I just found this CI tool: http://danger.systems
They have a similar idea, though it seems to be targeted at Ruby codebases.
In particular I like their clean summary:

A couple of ideas from them:

• Changelog entry (this gets forgotten a lot of times at Phobos, but with the new system there isn't an excuse anymore. We could show a warning that if no changelog entry nor bug report was provided (it can be ignored)
• CodeCov integration (seems to be hard to understand sometimes)
• Dscanner integration (people tend to have problems with analyzing the CircleCi output)
• Other linting tools we use at Phobos (e. g. parsing out all public unittests and running them with the module to ensure that they will be runnable on dlang.org)
• Tab/Whitespace (still happens from time to time)

Other ideas to check for:

• regression fixes should target "stable"
• PR description is too short (leads often too unnecessary stalling if the PR)

For all warnings / errors of course we should provide links with a detailed explanation.

CCing @JackStouffer for more ideas as he has done quite a bit of reviews at Phobos over the last year.

[wilzbach]

Especially forgetting to point bug fixes at stable happens quite often: dlang/tools#218

[JackStouffer]

• if the diff is over 400 lines, suggest to break the PR up into either smaller commits or separate PRs in order to facilitate faster reviews
• only show the "needs a change log entry" for fixes to enhancement requests on bugzilla