RFR: 1707: Bot warns about "No .jcheck/conf found" when it should not [v4]

Magnus Ihse Bursie ihse at openjdk.org
Tue Dec 6 10:40:15 UTC 2022


On Mon, 5 Dec 2022 23:55:09 GMT, Zhao Song <zsong at openjdk.org> wrote:

>> In [SKARA-1393](https://bugs.openjdk.org/browse/SKARA-1393), I added some checks to ensure the target branch of a pr contains valid jcheck configuration. This change went live on November 28, however, some users found that the warning of "No .jcheck/conf found" printed after they integrated their pr. After more investigation, I am thinking it's maybe a GitLab bug. Since when the issue happens, GitLab will always return "Commit Not Found", so we could have a temporary workaround right now.
>> 
>> For GitLab REST API:
>> If commit not found, it will return "404 Commit Not Found"
>> 
>> If file not found it will return "404 File Not Found"
>> 
>> For Github REST API:
>> If commit not found, it will return "No commit found for the ref "
>> 
>> If file not found it will return "Not Found"
>
> Zhao Song has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fix a problem

Apart from the naming issue I think this looks good, but please await Erik's review as well.

forge/src/main/java/org/openjdk/skara/forge/github/GitHubRepository.java line 265:

> 263:     @Override
> 264:     public Optional<String> fileContents(String filename, String ref) {
> 265:         JSONValue conf;

That's a bit odd name for the variable. I realize you did not come up with it, but I think it is a remnant from the time this function was only used to read the configuration file. I recommend changing it to `content`.

forge/src/main/java/org/openjdk/skara/forge/gitlab/GitLabRepository.java line 291:

> 289:     public Optional<String> fileContents(String filename, String ref) {
> 290:         var confName = URLEncoder.encode(filename, StandardCharsets.UTF_8);
> 291:         JSONValue conf = null;

The same here. Also update `confName`, perhaps `encodedFileName`?

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

Marked as reviewed by ihse (Reviewer).

PR: https://git.openjdk.org/skara/pull/1435


More information about the skara-dev mailing list