RFR: Refactor and cleanup
erikj at openjdk.java.net
Thu Jun 10 23:36:05 UTC 2021
On Thu, 10 Jun 2021 21:27:17 GMT, Erik Joelsson <erikj at openjdk.org> wrote:
> 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.
I reverted the text block change and the bump of the required JDK version to 15. If we are to do that, it should be in a proper change.
More information about the skara-dev