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

Deprecate space assignment syntax for Groovy #31424

Merged
merged 5 commits into from
Dec 4, 2024

Conversation

ov7a
Copy link
Member

@ov7a ov7a commented Nov 26, 2024

Fixes: #31413

Note: the relevant commit is

The suppression for smoke-tested plugins is in

  • f794530
    (SantaTracker, Build Scans, Nebula Plugin)

The last two commits are docs and tests changes.

Reviewing cheatsheet

Before merging the PR, comments starting with

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

@ov7a ov7a self-assigned this Nov 26, 2024
@ov7a
Copy link
Member Author

ov7a commented Nov 26, 2024

@bot-gradle test this

@bot-gradle

This comment has been minimized.

@bot-gradle
Copy link
Collaborator

The following builds have failed:

@ov7a ov7a force-pushed the ov7a/deprecate-space-assignment branch 4 times, most recently from 033b613 to 3482f89 Compare November 27, 2024 17:44
@ov7a ov7a marked this pull request as ready for review November 28, 2024 08:11
@ov7a ov7a requested review from a team and ldaley as code owners November 28, 2024 08:11
@ov7a ov7a requested review from octylFractal, hegyibalint, bamboo and alllex and removed request for a team November 28, 2024 08:11
@bamboo bamboo removed their request for review November 28, 2024 12:39
@cobexer cobexer removed the request for review from a team November 28, 2024 15:19
@ov7a ov7a force-pushed the ov7a/deprecate-space-assignment branch from 3482f89 to f794530 Compare November 29, 2024 11:43
@mlopatkin mlopatkin requested review from mlopatkin and removed request for alllex and a team December 2, 2024 14:29
Copy link
Member

@mlopatkin mlopatkin left a comment

Choose a reason for hiding this comment

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

Great work! Thank you for fixing buildDir uses along the way.

I must say, it would be significantly easier to review the PR if purely mechanical changes of adding = without changing anything else were in a separate PR. The PR this big gave hard time to my browser.

❓ It looks like AGP has plenty of snippets that rely on the space-assignment syntax, for example, here. We might want to reach out to them.

There are some stylistic issues in public docs, that, I think, are worth addressing before merging. I'm less concerned about tests, though inconsistent use of uri(..) and file(..) gives me an itch. But the time to fix that is better spent elsewhere.

@ov7a ov7a requested a review from mlopatkin December 3, 2024 16:04
Copy link
Member

@mlopatkin mlopatkin left a comment

Choose a reason for hiding this comment

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

I think we can move forward with that.

@@ -8,20 +8,20 @@ repositories {
}

android {
compileSdkVersion 30
compileSdkVersion = 30
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
compileSdkVersion = 30
compileSdk = 30

@ov7a ov7a force-pushed the ov7a/deprecate-space-assignment branch from d7d0ef2 to a5999d2 Compare December 3, 2024 21:36
@ov7a ov7a added this pull request to the merge queue Dec 3, 2024
@bot-gradle bot-gradle added this to the 8.12 RC1 milestone Dec 3, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 3, 2024
@ov7a ov7a force-pushed the ov7a/deprecate-space-assignment branch from a5999d2 to 2320428 Compare December 4, 2024 07:26
@ov7a ov7a added this pull request to the merge queue Dec 4, 2024
Merged via the queue into master with commit 17ad6c6 Dec 4, 2024
18 checks passed
@ov7a ov7a deleted the ov7a/deprecate-space-assignment branch December 4, 2024 08:36
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.

Deprecate "space-assignment" syntax in Groovy
3 participants