Integrated: Refactor and cleanup

Erik Joelsson erikj at openjdk.java.net
Fri Jun 11 19:52:21 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.

This pull request has now been integrated.

Changeset: 9371c001
Author:    Erik Joelsson <erikj at openjdk.org>
URL:       https://git.openjdk.java.net/skara/commit/9371c0019dfa81cbdf69f5d421da9748a5c8228a
Stats:     158 lines in 6 files changed: 46 ins; 62 del; 50 mod

Refactor and cleanup

Reviewed-by: kcr

-------------

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


More information about the skara-dev mailing list