RFR: WIP/RFC: added ProblemLists check to jcheck

Erik Helin ehelin at openjdk.java.net
Thu Mar 19 23:53:24 UTC 2020


On Thu, 19 Mar 2020 14:27:24 GMT, Igor Ignatyev <iignatyev at openjdk.org> wrote:

>> Hi Igor,
>> 
>> thanks for contributing ��
>> 
>> We are in general very positive to bring in additional "jchecks" to upstream since they can easily be configured per
>> repository. The different OpenJDK projects usually choose the checks they want enabled/disabled themselves.
>> I think the check you are proposing makes a lot of sense, I've run into this scenario myself in the past. I had a brief
>> look at the code, will do a more in-depth review later, but at a first glance it looks really good �� One thing that
>> stands out: could you please add one or more unit tests for your new check? You can use the unit tests for other checks
>> as an inspiration, please see the tests in
>> [jcheck/src/test/java/org/openjdk/skara/jcheck](https://github.com/openjdk/skara/tree/master/jcheck/src/test/java/org/openjdk/skara/jcheck).
>> Thanks, Erik
>
> 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

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

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


More information about the skara-dev mailing list