RFR: WIP/RFC: added ProblemLists check to jcheck

Igor Ignatyev iignatyev at openjdk.java.net
Fri Mar 20 00:03:08 UTC 2020


On Thu, 19 Mar 2020 15:13:22 GMT, Erik Helin <ehelin at openjdk.org> wrote:

>> Hi Erik,
>> 
>> Thanks for your prompt response. One of the reasons I haven't written any tests for this check is that I 1st wanted to
>> get opinions on whether such a check will be accepted to skara/jcheck as my previous experience with jcheck.py showed
>> that it's near to impossible to extend it with any new checks. Another reason is that this check reads the content of
>> files in a repo, which AFAICT none of the existing tests do. Anyhow, I'll work on developing tests.  Cheers,
>> -- Igor
>
> Ah ok, I get it. Skara's jcheck is very extensible, it is perfectly fine for a repository to start requiring more
> checks after a given commit by updating .jcheck/conf. It is also fine for a repository to only utilize a subset of the
> available checks (this is very common in practice).  Yes and no, the other checks do read at least .jcheck/conf �� What
> you need to be careful with is _which_ revision you read the file contents from. You probably want to use
> `ReadOnlyRepository::lines` instead of reading a file directly from disk (since a user might jcheck an earlier commit
> than the one currently being checked out). It should be fairly easy writing a unit test showcasing this scenario ��
> Thanks, Erik

Hi Erik,

thanks for the hint, I've updated the check to use `ReadOnlyRepository` to read files' content and list files in a
directory. I've also added unit tests to cover different scenarios and configuration combinations.

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?

BTW, should I update copyright years in all the touched files?

Thanks,
-- Igor

-------------

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


More information about the skara-dev mailing list