RFR: 2226: Make it possible to avoid "forward backports"
Zhao Song
zsong at openjdk.org
Fri Apr 12 18:18:40 UTC 2024
On Fri, 12 Apr 2024 08:54:03 GMT, Erik Duveblad <ehelin at openjdk.org> wrote:
> Hi all,
>
> please review this re-implementation of avoiding so called "forward backports", this time around in the `IssueNotifier`, meaning that Skara will avoid to create a "forward backport" _after_ a pull request has been integrated. Solving the problem in the `IssueNotifier` turned out to be a much more natural fit, so thanks @zhaosongzs and @erikj79 for the discussion in #1630.
>
> The new implementation will avoid creating a "foward backport" if _no_ suitable backport (or "forward backport") is found. That is, if JBS already contains a backport (or "forward backport") that can be used, then that backport (or "forward backport") will be used. This makes sense to me because in this scenario the user has (most likely) set up the issues in JBS for this to happen. But when Skara decides to create a _new_ backport, then (if `"avoidforwardports"` is `true` in the configuration) the code will never create "forward backport", instead opting to create a backport with the primary issue's `fixVersions` and setting `fixVersions` of the primary issue to the value found in the repository's `.jcheck/conf`.
>
> I also added a number of new unit tests. Note that this pull request depends on #1636 in order for the tests to pass, so the tests will fail until #1636 is integrated.
>
> Thanks,
> Erik
Overall this patch looks good!
I believe the issueNotifier would work well even when the bot gets interrupted.
Will approve it after SKARA-2233 gets integrated and all the GitHub actions pass.
bots/notify/src/test/java/org/openjdk/skara/bots/notify/issue/IssueNotifierTests.java line 2330:
> 2328: issue.setProperty("priority", JSON.of("1"));
> 2329: issue.addLabel("test");
> 2330: issue.addLabel("temporary");
In this case, maybe it's better to set the main issue as RESOLVED
bots/notify/src/test/java/org/openjdk/skara/bots/notify/issue/IssueNotifierTests.java line 2462:
> 2460: issue.addLabel("temporary");
> 2461:
> 2462: // Create an explicit "forwardport"
Should be `Create an explicit "backport"`
-------------
PR Review: https://git.openjdk.org/skara/pull/1637#pullrequestreview-1998153887
PR Review Comment: https://git.openjdk.org/skara/pull/1637#discussion_r1562975306
PR Review Comment: https://git.openjdk.org/skara/pull/1637#discussion_r1562978528
More information about the skara-dev
mailing list