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.

Sign up for GitHub

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

Jump to bottom

Polish GradleLifecycle implementation #28957

Merged
merged 4 commits into from Apr 25, 2024
Merged

Polish GradleLifecycle implementation #28957

merged 4 commits into from Apr 25, 2024

Conversation

bamboo
Copy link
Member

@bamboo bamboo commented Apr 24, 2024

Follow-up to #28875

Reviewing cheatsheet

Before merging the PR, comments starting with

  • ❌ ❓must be fixed
  • 🤔 💅 should be fixed
  • 💭 may be fixed
  • 🎉 celebrate happy things

@bamboo
Polish SerializedLambdaQueries
2f372a7 
- Remove unused method
- Reorder methods for top-down reading
- Reduce type visibility
@bamboo
Polish DefaultGradle
b46eb42 
- Remove unnecessary `final` modifier
- Document why `DefaultGradleLifecycle` is instantiated via `ObjectFactory`
@bamboo
Polish DefaultIsolatedProjectEvaluationListenerProvider
f60f35b 
- Simplify `popSingletonProperty` by introducing
  `ExtraPropertiesExtension.remove`
- Move extension methods to their own files
@bamboo bamboo added a:chore Minor issue without significant impact in:isolated-projects 1 min review labels Apr 24, 2024
@bamboo bamboo added this to the 8.9 RC1 milestone Apr 24, 2024
@bamboo bamboo requested a review from alllex April 24, 2024 20:44
@bamboo bamboo self-assigned this Apr 24, 2024
@bamboo bamboo requested review from a team as code owners April 24, 2024 20:44
@bamboo bamboo requested review from mlopatkin and removed request for a team April 24, 2024 20:44
@bamboo
Copy link
Member Author

bamboo commented Apr 24, 2024

@bot-gradle test

@bot-gradle
Copy link
Collaborator

I've triggered the following builds for you. Click here to see all build failures.

@alllex
Copy link
Member

alllex commented Apr 24, 2024

While at it, could you also fix the javadoc for IsolatedProjectEvaluationListenerProvider.clear and add the service scope to that interface? I believe it should be @ServiceScope(Scope.Build.class).

Also, related to this service I noticed this: https://github.com/gradle/gradle/pull/28581/files#r1578518821

@bamboo
Copy link
Member Author

bamboo commented Apr 25, 2024

@bot-gradle merge

@bot-gradle bot-gradle added this pull request to the merge queue Apr 25, 2024
@bamboo
Copy link
Member Author

bamboo commented Apr 25, 2024

Thanks, @alllex!

alllex
alllex reviewed Apr 25, 2024
View reviewed changes
...he/src/main/kotlin/org/gradle/configurationcache/extensions/ServiceRegistrationExtensions.kt
* @see [ServiceRegistration.add]
*/
internal
inline fun <reified ServiceType, reified ImplementationType : ServiceType> ServiceRegistration.add() {
Copy link
Member

Choose a reason for hiding this comment

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

👍

Merged via the queue into master with commit 6ac6f7e Apr 25, 2024
21 checks passed
@bot-gradle bot-gradle deleted the bamboo/ip/polish-GradleLifecycle/i branch April 25, 2024 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alllex alllex alllex approved these changes

@mlopatkin mlopatkin Awaiting requested review from mlopatkin mlopatkin is a code owner automatically assigned from gradle/bt-configuration

Assignees

@bamboo bamboo

Labels
1 min review 10 min review a:chore Minor issue without significant impact in:isolated-projects
Projects
None yet
Milestone
8.9 RC1
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@bamboo @bot-gradle @alllex