RFR: 1696: CSRBot uses wrong fixVersion when resolving CSR issues for PR [v2]

Erik Joelsson erikj at openjdk.org
Mon Dec 5 15:16:48 UTC 2022

> When CSRBot looks for a suitable issue in a set of Backports and main bug, it gets the fixVersion from the target branch of the PR. This is conceptually wrong, as the PR may be including a change to the fixVersion, and if that is the case, any associated bug should be resolved with fixVersion set to the value the PR changed it to.
> We currently have this PR where exactly that is happening:
> https://github.com/openjdk/jdk/pull/10924
> The associated bugs have fixVersion correctly set to 21, as this is the initial change for bumping the JDK version from 20 to 21. This is causing CSRBot to not be able to find the linked CSR issues.
> The same kind of logic is also present in PullRequestBot, and unfortunately the methods are copied. The reason for this is the lack of a common place to put application level shared logic for multiple bots. 
> This patch changes the getVersion logic to first try to get it from the sourceHash of the PR and if that fails, fallback to the targetRef like before. I don't think this will fail much in practice, but there could be corner cases where it might be possible. The failure condition here is catching `UncheckedRestException` and checking the return code. This isn't ideal, and that particular exception is a bit of an implementation detail. Maybe we should consider providing a better API for getting file contents that may be missing?
> I'm introducing a new module with this change where common bot logic can be housed, and put the updated version extraction logic there. This has been missing for a long time IMO and there is a lot of possible cleanup possible after introducing this module. Several modules have similar classes doing almost the same thing. I also think that some of the "utils" logic in the forge module could find a better home here.

Erik Joelsson has updated the pull request incrementally with one additional commit since the last revision:

  Fix another test


  - all: https://git.openjdk.org/skara/pull/1429/files
  - new: https://git.openjdk.org/skara/pull/1429/files/18a84ae5..e2c8ab4e

 - full: https://webrevs.openjdk.org/?repo=skara&pr=1429&range=01
 - incr: https://webrevs.openjdk.org/?repo=skara&pr=1429&range=00-01

  Stats: 8 lines in 1 file changed: 0 ins; 4 del; 4 mod
  Patch: https://git.openjdk.org/skara/pull/1429.diff
  Fetch: git fetch https://git.openjdk.org/skara pull/1429/head:pull/1429

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

More information about the skara-dev mailing list