RFR: 1236: Add jcheck option to check for large binary files

Guoxiong Li gli at openjdk.java.net
Wed Dec 1 11:37:51 UTC 2021

On Tue, 30 Nov 2021 22:56:56 GMT, Erik Joelsson <erikj at openjdk.org> wrote:

> I agree with Kevin, I don't think we have any need for configuring different file types having different size requirements.

If no one agrees with me or gives another idea these days, I will follow this idea.

> It looks like you have taken the existing "binary" check and changed it to be a size check. 

Because when I dove into the class `PullRequestCheckIssueVisitor`, I fould that the `BinaryIssue` is never used and the check result has no effect in the PR. So I think `BinaryIssue` is not necessary and rename it to `BinaryFileTooLargeIssue`.

// class PullRequestCheckIssueVisitor
    public void visit(BinaryIssue issue) {
        log.fine("ignored: binary file");
        // <---- miss to add the failed message to the `failedChecks`.

But now I find the `JCheckCLIVisitor` use the `BinaryIssue` which means the `BinaryIssue` is effective. So now I think the `PullRequestCheckIssueVisitor` just has a small bug that it miss to add the failed message to the `failedChecks`. And actually, the result of the `binary` check in the PR has never activated in the past.

// class JCheckCLIVisitor
    public void visit(BinaryIssue i) {
        if (!ignore.contains(i.check().name())) {
            println(i, "adds binary file: " + i.path().toString());
            hasDisplayedErrors = true;

I will solve this small bug of the `PullRequestCheckIssueVisitor` in this patch and will keep the `BinaryIssue` instead of removing `BinaryIssue`.


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

More information about the skara-dev mailing list