Skip to content

Polish the Gradle build's configuration phase #5610

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

shanman190
Copy link
Contributor

@shanman190 shanman190 commented Jun 12, 2025

What's changed?

Gradle build cleanup.

Before: 1m 58s
After: 18s

What's your motivation?

Get to actually having Gradle do work sooner

Anything in particular you'd like reviewers to focus on?

rewrite-javascript had a bit of restructuring, but is fundamentally the same. It should be reviewed carefully though.

Several of the previous rewrite-javascript tasks were using the node-gradle plugin's internal Gradle rule (ie. npm_* which resulted in eager task creation).

Anyone you would like to review specifically?

Anybody

Have you considered any alternatives or workarounds?

N/A

Any additional context

There are additional pieces that can be cleaned up. There are several plugins -- the license plugin being the worst offender -- that still eagerly create tasks and we could investigate next enabling configuration cache as well.

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've read and applied the recipe conventions and best practices
  • I've used the IntelliJ IDEA auto-formatter on affected files

@shanman190
Copy link
Contributor Author

I'll investigate the build failure for rewrite-javascript. My bet is that it's missing a npmInstall task dependency.

}

tasks.register<Test>("npmTestReporting") {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gradle has an npm agent in beta that will allow for reporting of the tests to Develocity.
https://docs.gradle.com/develocity/npm-agent/

This'll achieve the goal where the desire was to publish npm tests alongside the build scan.

@github-project-automation github-project-automation bot moved this from In Progress to Ready to Review in OpenRewrite Jun 13, 2025
@timtebeek
Copy link
Member

@sambsnyd / @jkschneider this seems a clear performance boost to the builds and the responsiveness in the IDE, but of course we've not been able to test drive the publication for rewrite-javascript while on a branch. Are you comfortable merging this in and watch the snapshot publish from there? Or would you rather hold off until a better moment as to not get in the way of being able to release at any moment?

@shanman190
Copy link
Contributor Author

shanman190 commented Jun 13, 2025

I'll plan to fix the merge conflicts from the recent JS changes from Knut in order to make this PR mergable again. This'll include optimizing any newly introduced tasks from Knut's PR as well.

@timtebeek timtebeek moved this from Ready to Review to In Progress in OpenRewrite Jun 14, 2025
@shanman190 shanman190 force-pushed the perf/gradle-configuration-phase branch from b63e7ec to 710c966 Compare June 18, 2025 20:33
@shanman190
Copy link
Contributor Author

Alright, I ended up reapplying my first commit in isolation without rewrite-javascript on top of the latest main. Then redoing the rewrite-javascript updates because there were a few too many changes to keep the direct merge straight for.

I did opt to take rewrite-javascript a little bit further with this latest refactor, but there's still a lot more that can be done.

@shanman190 shanman190 force-pushed the perf/gradle-configuration-phase branch from 8647dea to 9cb1f11 Compare June 18, 2025 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

3 participants