RFR: 84: Default bug ID format should match the JDK default

[Erik Duveblad via github.com]

On Thu, 29 Aug 2019 06:13:45 GMT, Kevin Rushforth via github.com <duke at openjdk.java.net> wrote:

> On Thu, 29 Aug 2019 06:13:44 GMT, Kevin Rushforth via github.com <duke at openjdk.java.net> wrote:
> 
>> Fix for [SKARA-84](https://bugs.openjdk.java.net/browse/SKARA-84) to change the default issue pattern to require exactly 7 digits with no prefix to match `hg jcheck`.
>> 
>> I updated all of the tests to account for this and added new tests for now-invalid formats.
>> 
>> ----------------
>> 
>> Commits:
>>  - 4e819911:	84: Default bug ID format should match the JDK default
>> 
>> Pull request:
>> https://git.openjdk.java.net/skara/pull/99
>> 
>> Webrev:
>> https://webrevs.openjdk.java.net/skara/99/webrev.00
>> 
>> Patch:
>> https://git.openjdk.java.net/skara/pull/99.diff
>> 
>> Fetch command:
>> git fetch https://git.openjdk.java.net/skara pull/99/head:pull/99
> 
> I filed [SKARA-86](https://bugs.openjdk.java.net/browse/SKARA-86) to associate my GitHub username, `kevinrushforth`, with my OpenJDK username, `kcr`.
> 
> PR: https://git.openjdk.java.net/skara/pull/99

Hi Kevin,

first of all, thanks for contributing! Unfortunately there are dependencies on the regex your are changing in the rest of the code, particularly in the `HgToGitConverter` (via `ConverterCommitMessageParser`). That means that your change will cause the conversion for some OpenJDK Mercurial repositories to start to fail, so we can't integrate this patch as is :(

I think a better approach here would be to implement https://bugs.openjdk.java.net/projects/SKARA/issues/SKARA-80, but _not_ at the `vcs` level (`vcs` is one "layer" below the `jcheck` layer). Instead I would recommend enhancing git-jcheck's configuration file to allow customizing the `issue` check, for example:
```
[checks "issues"]
pattern = ^([1-9][0-9]{6}): (\\S.*)$
```
Then you could extend `ChecksConfiguration` to have an `IsssuesConfiguration` member (see for example how `ReviewersConfiguration` or `WhitespaceConfiguration` works). Finally you would then have to update the `IssuesCheck` to read this configuration and the pattern and check that all issues in `message.issues()` conforms to the pattern, for example:
```
var issues = new ArrayList<Issue>();
var patttern = Patter.compile(conf.checks().issues().pattern());
for (var issue : message.issues()) {
    if (!pattern.matcher(issue.toString()).matches()) {
        issues.add(new IssuesIssue(metadata));
    }
}
return issues.iterator();
```
The last part that would then fulfill what you want this patch to do would be have the default value for `pattern` in `IssueConfiguration` be `"^([1-9][0-9]{6}): (\\S.*)$"`.

To try to make the architecture a bit more clear: the `vcs` layer should work for _all_ OpenJDK Projects, not just the JDK Project. Then projects can specify how they want jcheck to work for their project via .jcheck/conf. The good news is that you can still re-use all the updates you made to the unit tests for a patch implementing the above!

PR: https://git.openjdk.java.net/skara/pull/99