-
-
Notifications
You must be signed in to change notification settings - Fork 101
build: improve perf parse of versions plugin #2479
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution. It looks very interesting.
There are a couple of things to keep in mind when you send a PR, though:
- Please, apply our code templates. You can find them here: https://github.com/hibernate/hibernate-reactive
We use the same one as ORM. - Try to keep the changes in a commit to a minimum, this makes it easier for me to review them.
In particular, don't change the code format, and try to keep the non important changes in a separate commit. For example, you have changed the indentation from tab to spaces, and some variable types fromObjecttovar. These are all stylistic choice that don't affect the actual fix. They should, at the very least, be in a separate commit. - Try to explain in the commit messages why that commit is required or improves the build
- We usually don't add the
@authortag anymore in the javadocs. With multiple people working on the same code base, it becomes obsolete quickly and we would end up with a huge list of names in every class. We get the same information by checking the author in the commit
Thanks again!
local-build-plugins/src/main/java/org/hibernate/reactive/env/VersionsPlugin.java
Outdated
Show resolved
Hide resolved
With this change remove manual parser of libs.versions.toml file and use java Properties API. Additional retrieve data of the file with PropertiesSource of gradle to be ready with configuration cache.
27f3523 to
0ce1b0a
Compare
Fixes, I forgot applying project code style in IJ IDEA, I think is already in the idea folder. I rollback the use of var.
Thanks a lot, much better. I decided to apply a few changes, can you have a look at them and let me know what you think?
They are in this commit: DavideD@5414a2b
Nothing major, just some stylistic changes and some refactoring to remove duplicated code.
Superseded by #2479
Hi, based on configuration cache valid inputs I do a refactor on versions plugins in your local plugins.
Other suggestions, but out of scope of this PR:
In root
build.gradlecan use libs, explicit () and "":plugins { id( "local.versions" ) id( "java-library" ) id( "maven-publish" ) alias( libs.plugins.com.diffplug.spotless ) alias( libs.plugins.org.asciidoctor.jvm.convert ) apply false } group = "org.hibernate.reactive" // leverage the ProjectVersion which comes from the `local.versions` plugin version = project.projectVersion.fullName rootProject.afterEvaluate { logger.lifecycle( "ORM version: ${libs.versions.hibernateOrmVersion.get()}" ) logger.lifecycle( "ORM Gradle plugin version: ${libs.versions.hibernateOrmGradlePluginVersion.get()}" ) logger.lifecycle( "Vert.x SQL Client version: ${libs.versions.vertxSqlClientVersion.get()}" ) }In root
gradle.propertiesadding those reduce to 30s build time, similar to hibernate orm (properties, not build time xD)I recommend use version catalog for jdks version, like hibernate orm do.
Without further ado, if you can try it and give me feedback :)