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

[Gradle] Handle plugin version variables without string interpolation #5381

Merged
merged 3 commits into from
Sep 27, 2022
Merged

[Gradle] Handle plugin version variables without string interpolation #5381

merged 3 commits into from
Sep 27, 2022

Conversation

Flexicon
Copy link
Contributor

Fixes #4127

This change allows direct variables in place of versions to be used in Gradle Plugins scope without the need to redundantly interpolate them in a string.


This PR addresses the following issue:

Does not work - versions are ignored by Dependabot:

plugins {
    val kotlinVersion = "1.7.0"
    kotlin("jvm") version kotlinVersion
    kotlin("plugin.spring") version kotlinVersion
}

Works as a workaround - versions are picked up correctly:

plugins {
    val kotlinVersion = "1.7.0"
    kotlin("jvm") version "$kotlinVersion"
    kotlin("plugin.spring") version "$kotlinVersion"
}

@Flexicon Flexicon requested a review from a team as a code owner July 14, 2022 11:19
@Flexicon
Copy link
Contributor Author

@brrygrdn any chance to get some eyes on this one? 🙏

@jeffwidman jeffwidman added the L: java:gradle Maven packages via Gradle label Sep 22, 2022
Copy link
Member

@jeffwidman jeffwidman left a comment

Choose a reason for hiding this comment

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

👋 Hey there, sorry for the slow review.

This is great, super straightforward and nice job with the tests.

Can you rebase to fix the merge conflict?

@Flexicon
Copy link
Contributor Author

Done @jeffwidman

@jeffwidman
Copy link
Member

Thanks! Looks like tests are still failing though?

Also, can you explain why the tests need the length changed? I assumed originally that was related to a regex change capturing an additional character which gets parsed out, but since it's no longer doing that, makes me realize I don't know why the length change is/isn't needed?

@Flexicon
Copy link
Contributor Author

Yeah I see that, I'll take a look later and try and fix it. It's been a few months so my memory is a bit hazy.

I assumed originally that was related to a regex change capturing an additional character which gets parsed out

That was exactly the intention at the time. I'll have to dig in and check why it's not working anymore.

@Flexicon
Copy link
Contributor Author

Hey @jeffwidman, finally had some time to look into it. Turns out I messed up on the rebase and had a question mark in the wrong place for the regex.

Should be good to go now 👌

@Flexicon Flexicon requested a review from jeffwidman September 26, 2022 07:37
@Flexicon
Copy link
Contributor Author

@jeffwidman @bdragon any chance for another review 🙏🏻 The MR had to be updated with master again.

Copy link
Member

@Nishnha Nishnha left a comment

Choose a reason for hiding this comment

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

This makes sense to me.
Thanks!

@Nishnha Nishnha merged commit 8ab4d78 into dependabot:main Sep 27, 2022
@Flexicon Flexicon deleted the flexicon/handle-gradle-plugin-version-variables branch September 27, 2022 16:43
@jeffwidman
Copy link
Member

Sorry @Flexicon I was offline the past few days. Looks like @Nishnha has you covered though!

@pavera pavera mentioned this pull request Oct 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L: java:gradle Maven packages via Gradle
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for variables in plugins block
3 participants