Skip to content

Conversation

DavideD
Copy link
Member

@DavideD DavideD commented Sep 2, 2025

Fix #2483

Supersedes #2479

kingg22 and others added 2 commits September 2, 2025 10:30
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.
@kingg22
Copy link
Contributor

kingg22 commented Sep 2, 2025

LGTM.

I will try to enable configuration cache and fix the local plugins is not adapted to it, or you going to do that soon?

Suggestion: it’s better to create a follow-up PR after mine has been merged (a sequence of PRs), rather than opening a new PR that supersedes mine. That way, as an external contributor, I don’t lose the recognition of having a PR merged into a large public repository :)

@DavideD
Copy link
Member Author

DavideD commented Sep 2, 2025

I kept your original commit, so you will still be recognized. But if you prefer, feel free to get my changes and create a new PR. I don't mind either way (just please, add the issue in the commit messages so that I can collect the PR I will apply on other branches).

@kingg22
Copy link
Contributor

kingg22 commented Sep 2, 2025

@DavideD Don't worry, it's just a suggestion. You can merge however you prefer, and if it's fast, even better, so I can work on the other plugins to enable configuration cache ;)

@DavideD
Copy link
Member Author

DavideD commented Sep 2, 2025

By the way, I did that because I didn't want to bother you with small changes.

I will try to enable configuration cache and fix the local plugins is not adapted to it, or you going to do that soon?

No, starting tomorrow, I will have some time off and I have some other things to finish today. Feel free to send a PR and I will have a look at it as soon as I come back (if nobody else looks at it first).

@DavideD
Copy link
Member Author

DavideD commented Sep 2, 2025

It's probably better if you create an issue describing what problem you want to solve first, so that when you send the PR you can bind it to it. It's mainly for me be able to group PRs and keep track of what's going on. And this way you can add a an issue code to the commit message.

Thanks, I will merge this one soon.

@kingg22
Copy link
Contributor

kingg22 commented Sep 2, 2025

Fine, I will do it later. Have a good day

@DavideD DavideD merged commit f2e7ec0 into hibernate:main Sep 2, 2025
19 checks passed
@DavideD
Copy link
Member Author

DavideD commented Sep 2, 2025

Merged, thanks a lot @kingg22

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.

Cache the project version during the gradle build
2 participants