Skip to content

Conversation

jvandort
Copy link
Member

@jvandort jvandort commented Jul 7, 2022

My current task had me looking at this class and I saw some places that could use a bit of cleanup and reorganization.

  • Removed unused method parameters
  • Reduce usage of configuration name string constants and needless querying of configurations when we already had references elsewhere in class
  • Make some methods static where possible
  • Consolidate some short and related methods
  • Break up generic methods by moving code into relevant locations
  • Comment and javadoc formatting

@jvandort jvandort added this to the 7.6 RC1 milestone Jul 7, 2022
@jvandort jvandort requested a review from a team July 7, 2022 21:14
@jvandort jvandort self-assigned this Jul 7, 2022
@jvandort jvandort marked this pull request as ready for review July 8, 2022 14:53
Copy link
Member

@tresat tresat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these refactoring changes are a good start towards improving this class, but I have some questions and suggestions.

@jvandort jvandort requested a review from tresat July 12, 2022 18:51
@jvandort jvandort force-pushed the jvandort/rework-java-plugin branch from 10516ee to 07ac5c0 Compare July 13, 2022 16:51
My current task had me looking at this class and I saw some places that could use a bit of cleanup and reorginization.

* Removed unused method parameters
* Reduce usage of configuration name string constants and needless querying of configurations when we already had references elsewhere in class
* Make some methods static where possible
* Consolidate some short and related methods
* Break up generic methods by moving code into relevant locations
* Comment and javadoc formatting
* Move test code into test suites block, use configuration names from test source set instead of static constants
* Separate `configureConfigurations` into two methods, each creating one of `apiElements` and `runtimeElements`
@jvandort jvandort force-pushed the jvandort/rework-java-plugin branch from 07ac5c0 to e1c5f0e Compare July 13, 2022 16:52
@gradle gradle deleted a comment from jvandort Jul 13, 2022
@bot-gradle
Copy link
Collaborator

OK, I've already triggered a build for you.

@bot-gradle
Copy link
Collaborator

Pre-tested commit build failed.

@blindpirate
Copy link
Collaborator

The failure seems to be my fault, looking.

@blindpirate
Copy link
Collaborator

@bot-gradle test and merge

@gradle gradle deleted a comment from jvandort Jul 14, 2022
@gradle gradle deleted a comment from jvandort Jul 14, 2022
@bot-gradle
Copy link
Collaborator

OK, I've already triggered a build for you.

@bot-gradle bot-gradle merged commit 545bd90 into master Jul 14, 2022
@blindpirate blindpirate deleted the jvandort/rework-java-plugin branch July 14, 2022 05:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants