RFR: 2226: Make it possible to avoid "forward backports"
Erik Duveblad
ehelin at openjdk.org
Thu Apr 11 08:37:01 UTC 2024
On Tue, 9 Apr 2024 21:03:32 GMT, Erik Duveblad <ehelin at openjdk.org> wrote:
> Hi all,
>
> please review this patch that makes it possible to configure the PR bot to avoid creating a "forward backport" when integrating a pull request. A "forward backport" gets created when the issue being fixed a PR has a `fixVersion` that is less than the `fixVersion` for the repository (which is specified by `.jcheck/conf`). This patch detects this scenario and instead sets the `fixVersion` for the issue to the repository's `fixVersion` and creates a backport with the `fixVersion` of the original issue. Avoiding "forward backport"s is configurable and will only be enabled for the [jdk](https://github.com/openjdk/jdk) repository (it would be a bug to have this behaviour for update release repositories).
>
> For the patch itself, there are two larger changes that are almost just moving code: the creation of the new class `OverridingJCheckConfigurationParser` and the addition of `createBackport` to `Backports` (and the removal of `JbsBackports`). `OverridingJCheckConfigurationParser` is useful for parsing a `.jcheck/conf` that can be overridden _without_ a local repository (only with a `HostedRepository`). `Backports.createBackport` is almost a clean refactoring of `JbsBackport` - no functionality has been changed.
>
> There is one detail in the patch that might look weird:
>
> issue.get().setProperty("fixVersions",
> JSON.array().add(targetFixVersion.get().raw()));
>
> In `IssueNotifier` the code is setting `fixVersions` to a value of a single JSON string. However, this value gets transformed by `JiraProject.encodeProperty`. The problem with not using an array (which the property value should be) is that tests gets confused (since `TestIssueTrackerIssue.setProperty` is *not* transforming its input). I therefore opted to use an array in the production code to make it easier to test.
>
> I also added a bunch of new unit tests as well as slightly updated the integration tests for backports.
>
> Thanks,
> Erik
@zhaosongzs and @erikj79 thanks for great reviews and discussion! I will split this work into two patches, one for [SKARA-2227 - Describe planned modifications to issues](https://bugs.openjdk.org/browse/SKARA-2227) and one for [SKARA-2226 - Make it possible to avoid "forward backports"](https://bugs.openjdk.org/browse/SKARA-2226).
The implementation of [SKARA-2226](https://bugs.openjdk.org/browse/SKARA-2226) will change to avoid the creation of "forward backports" _after_ a PR has been integrated (and yes, those changes will primarily be made to the `IssueNotifier`). As @erikj79 mentioned the concept of describing planned changes to issues is a general useful concept, so we can apply it to all issues (not only those that happen to have a fixVersiont that differs from the target repository's). That work will be done as part of [SKARA-2227](https://bugs.openjdk.org/browse/SKARA-2227).
I will close this PR since it no longer represents how we want to solve [SKARA-2226](https://bugs.openjdk.org/browse/SKARA-2226) and will open a new one PR with a solution focused on avoiding "forward backports" after a PR has been integrated.
-------------
PR Comment: https://git.openjdk.org/skara/pull/1630#issuecomment-2049197605
More information about the skara-dev
mailing list