Skip to content
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

Partially refactor to use more idiomatic Gradle #241

Merged
merged 12 commits into from
Jul 7, 2023

Conversation

3flex
Copy link
Contributor

@3flex 3flex commented Jul 5, 2023

The main changes are:

  1. Use managed properties, where possible
  2. Avoid passing instances of Project around
  3. Use the plugin to set default values in tasks and the extension

Parts 1 & 3 are not fully completed but I didn't want to press on if this wasn't desired. This should be safe to merge as is though as compat tests for Gradle 6 on Java 11 pass locally.

@3flex
Copy link
Contributor Author

3flex commented Jul 6, 2023

Looks like failure is due to #228

Copy link
Collaborator

@TWiStErRob TWiStErRob left a comment

Choose a reason for hiding this comment

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

Nice one! I'm curious too, @szpak, if this is a desired changeset to continue working on? Probably the best time for 2.0. I would be happy to help on this.

@TWiStErRob
Copy link
Collaborator

@3flex re #228, probably temporarily ignore the test that fails and observe green CI without them, just to be sure and then revert.

@szpak
Copy link
Contributor

szpak commented Jul 6, 2023

Nice one! I'm curious too, @szpak, if this is a desired changeset to continue working on? Probably the best time for 2.0. I would be happy to help on this.

I have to admit, I don't closely follow the recent changes in Gradle and how the "idiomatic Gradle" looks like in 2023 :-/. However, in general those changes look fine as simplifications and especially with the already mentioned Robert's branch, they would be nicely fit in the upcoming 2.0.0.

@TWiStErRob
Copy link
Collaborator

I think this is good as it is and can be merged, I'm happy to pick up and rebase my branch on top. I can also fix #241 (comment) there.

@szpak
Copy link
Contributor

szpak commented Jul 7, 2023

Thanks @3flex for the PR and @TWiStErRob for the code review!

I think this is good as it is and can be merged, I'm happy to pick up and rebase my branch on top. I can also fix #241 (comment) there.

Sound good.

@szpak szpak merged commit 5850613 into gradle-nexus:master Jul 7, 2023
6 checks passed
@3flex 3flex deleted the refactor branch July 7, 2023 22:18
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.

3 participants