RFR: 1854: Add "clean" label for merge PRs that have no merge resolution [v2]

Erik Joelsson erikj at openjdk.org
Wed Apr 12 19:50:04 UTC 2023


On Wed, 12 Apr 2023 17:27:50 GMT, Zhao Song <zsong at openjdk.org> wrote:

>> In this patch, PR bot would add clean label to Merge-style PRs if one of the following conditions is met.
>> 
>> 1. There isn't a merge commit at HEAD. 
>> To verify this condition, we can simply check the number of parents that the HEAD commit has. If the HEAD commit has one parent, it is not a merge commit. However, if it has two parents, then it is considered a merge commit.
>> 
>> 2. The merge commit at HEAD is empty(has no merge resolutions)
>> To verify this condition, we should be able to tell the difference between an empty merge commit and a non-empty merge commit.
>> 
>> Using `git show <Hash>` would help us differentiate between this two types of commits.
>> 
>> (1) git show output for empty merge commit
>> 
>> commit 9752b806edafbdc210db00f3099404cd8bd11edf
>> Merge: 982b5ad5d8b 99f8d05da93
>> Author: Zhao Song <zhao.song at oracle.com>
>> Date:   Tue Apr 11 14:26:18 2023 -0700
>> 
>>     Merge branch 'feature'
>> 
>> 
>> 
>> (2) git show output for non-empty merge commit
>> 
>> commit 242a0c07cf3204c662a2f1d55a2ca3e494f955fd (HEAD -> master)
>> Merge: af35791d093 0ebf3bd5fe1
>> Author: Zhao Song <zhao.song at oracle.com>
>> Date:   Tue Apr 11 14:38:05 2023 -0700
>> 
>>     Merge branch 'feature2'
>> 
>> diff --cc test.java
>> index 7c736ab8f08,09647d12eda..a3c5725e39d
>> --- a/test.java
>> +++ b/test.java
>> @@@ -3,4 -3,4 +3,5 @@@
>>   aaaaa
>>   +++++
>>   11111
>>  +33333
>> + 22222
>> 
>> 
>> 
>> We could find that the difference between the two is whether the commit message body is empty or not.
>> 
>> Since we only care about commit message body, we can use the `git show --pretty=format:%b <Hash>` command to extract solely the commit message body.
>
> Zhao Song has updated the pull request incrementally with one additional commit since the last revision:
> 
>   update the logic in updateMergeClean

bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java line 459:

> 457:                     }
> 458:                 }
> 459:             }

I think this is doing the right thing, but the abstraction isn't right. If we were to ever implement this for HG, then we can't know what the output would look like here, and we can't assume it will be the same as for Git. The knowledge about how to interpret the output of Git needs to be encapsulated in `GitRepository`. Most of this method should move into a new method in `Repository`/`GitRepository`.

Thinking some more about this, don't we have enough information in the `Commit` instance to figure this out? If the commit doesn't have any patches, then we should be good to go right? If so, we don't need any new method on `Repository`.

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

PR Review Comment: https://git.openjdk.org/skara/pull/1500#discussion_r1164567279


More information about the skara-dev mailing list