RFR: Refactor and cleanup

Erik Joelsson erikj at openjdk.java.net
Thu Jun 10 21:30:20 UTC 2021

I have been working quite a bit in the area of the Integrate and SponsorCommand classes recently, and while doing so noted that they have a lot of duplicate code. Some of that I fixed, but I didn't want to include unrelated refactoring and cleanup in a functional change, so here is a separate change with just that. Nothing should change from a functional perspective. This change only touches a limited set of classes where I had noted specific issues. Here is a breakdown of the changes.

1. Moved logic for finding the original change hash in a backport PR to the CheckablePullRequest class. All three locations where this was done already had an instance of this class, and that class has all the data needed for the operation, so seemed like a decent fit.
2. Put a lazily initialized wrapper of PullRequestUtils.targetHash(localRepo) in CheckablePullRequest. This method is called quite often, and each time it results in running a git command to resolve the hash from the ref name. When browsing logs, this git command is sprinkled all over the place so reducing it will improve both performance and log browsing. Unfortunately I can't figure out a good way to completely reduce it as PullRequestUtils is a static class and it calls this method quite a bit itself.
3. Moved a duplicate piece of code (for running and reacting to jcheck checks) in IntegrateCommand.handle() and SponsorCommand.handle() to a new static method in IntegrateCommand. I still didn't dare dabbling in a super class. 
4. Removed all dead parameters and variables that I could find in these classes.
5. Applied some other Intellij suggested code fixes, like removing exceptions never thrown and unneeded calls to toString(). I also let it convert a switch statements to the new fancy switch expression, which looks a lot better. Finally I also let it introduce a text block (this happened because I had left Intellij in JDK 16 mode). I think this looks good enough to warrant a bump in required JDK version for the project. We run everything default with JDK 16 already, and I can't see a reason to stick with 14. JDK 15 should be just as accessible.


Commit messages:
 - Refactor and cleanup

Changes: https://git.openjdk.java.net/skara/pull/1185/files
 Webrev: https://webrevs.openjdk.java.net/?repo=skara&pr=1185&range=00
  Stats: 167 lines in 7 files changed: 49 ins; 62 del; 56 mod
  Patch: https://git.openjdk.java.net/skara/pull/1185.diff
  Fetch: git fetch https://git.openjdk.java.net/skara pull/1185/head:pull/1185

PR: https://git.openjdk.java.net/skara/pull/1185

More information about the skara-dev mailing list