[Rev 02] RFR: jcheck: add "problemlists" check
ehelin at openjdk.java.net
Mon Mar 23 14:26:06 UTC 2020
On Mon, 23 Mar 2020 14:19:49 GMT, Erik Helin <ehelin at openjdk.org> wrote:
>> Igor Ignatyev has updated the pull request with a new target base due to a merge or a rebase. The pull request now
>> contains ten commits:
>> - Merge branch 'master' into master
>> - corrected copyright year
>> - added tests for ProblemListsCheck
>> - cleaned up imports
>> - ProblemListsIssue should use set to store information about files
>> - refiened default pattern in ProblemListsConfiguration
>> - updated ProblemListsCheck to list files in a dir via ReadOnlyRepository
>> - updated ProblemListsCheck to read files' content via ReadOnlyRepository
>> - added ProblemLists check to jcheck
> Looks good!
Great work on the patch and on the tests Igor, this is a really solid contribution!
As you probably noticed I took the liberty of editing the title of this pull request to align more with Skara's
(totally undocumented) conventions, I hope you don't mind?
> there is one thing which I would like to discuss before we (hopefully) get it integrated -- the default value for
> `dirs`. as `ProblemListsCheck` will most probably be used only by "jdk-based" project, it might make sense to use
> values which reflects jdk layout (`test/jdk|test/langtools|test/hotspot/jtreg|test/nashorn|test/jaxp`) so most projects
> will be able to use the default value, on the other hand, I don't want to bake in the current directory layout, so I
> defaulted `dirs` to `test` which thought isn't good for any of the existing OpenJDK projects. so what do you think?
I think defaulting to `test` is fine, I prefer that OpenJDK projects have to specify the directories containing
`ProblemList.txt` files in their `.jcheck/conf`. That makes it clear which files that will actually be considered.
> BTW, should I update copyright years in all the touched files?
More information about the skara-dev