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
@Override
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