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

Allow to use Action for configuration in extensions to better support non-Groovy consumers (#187) #404

Merged
merged 1 commit into from
Aug 22, 2024

Conversation

Vampire
Copy link
Contributor

@Vampire Vampire commented Aug 21, 2024

fixes #187

Copy link
Collaborator

@jeremylong jeremylong left a comment

Choose a reason for hiding this comment

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

LGTM

@jeremylong jeremylong merged commit 1e36cdb into dependency-check:main Aug 22, 2024
2 checks passed
@jeremylong
Copy link
Collaborator

Thanks!

@Vampire Vampire deleted the action-for-config branch August 22, 2024 11:13
@chadlwilson
Copy link
Contributor

How are users expected to use the Action variants in Groovy to avoid the deprecations?

Was it necessary to deprecate all the closure variants here?

@Vampire
Copy link
Contributor Author

Vampire commented Jan 12, 2025

Without deprecation it is hard to later remove them.
And in non-dynamic languages or if the Groovy files are statically compiled, you would otherwise get trouble when later removing the Closure variants as the already compiled code would not just "switch over" like the dynamic Groovy code would do.

In Groovy you have the two possibilities to either ignore the deprecation warning, or explicitly using the Action variant.

Or you switch over to Kotlin DSL instead. By now it is the default DSL, you immediately get type-safe build scripts, actually helpful error messages if you mess up the syntax, and amazingly better IDE support when using a good IDE like IntelliJ IDEA or Android Studio. :-)

@chadlwilson
Copy link
Contributor

I understand that re kotlin, but some of us have massive multi project Gradle builds and moving to kotlin is significant effort better spent elsewhere.

The question is around how to even use the action variant within groovy (the syntax). If there is no canonical (non clunky) way to do so, and if we are not deprecating support for groovy entirely, they probably shouldn't be marked as deprecated. Training folks to ignore deprecation warnings is not helpful.

Currently the documentation doesn't show folks how to use a non-deprecated approach.

@jeremylong
Copy link
Collaborator

Wouldn't it just be:

dependencyCheck {
    data.directory='d:/nvd'
    analyzers.assemblyEnabled=false
}

Basically just replace the {} with dot walking.

@Vampire
Copy link
Contributor Author

Vampire commented Jan 12, 2025

Well, this plugin's documentation should imho not be a Groovy language documentation, that is up to Groovy. ;-)
It is for example

dependencyCheck {
   proxy({
      it.port = 0
   } as Action)
}

Or you use neither by doing

dependencyCheck {
   proxy.port = 0
}

Besidest that, even with

dependencyCheck {
   proxy {
      port = 0
   }
}

Gradle does not complain with a deprecation warning for me.

@jeremylong
Copy link
Collaborator

I really should update the docs to be kotlin at some point...

@chadlwilson
Copy link
Contributor

chadlwilson commented Jan 13, 2025

Ahh thanks, despite many years of using Gradle it wasn't obvious to me that moving to dot syntax is what it takes to move to using the actions overloads. I guess this is the difference from the perspective of users vs Gradle plugin authors; as the closure syntax is the canonical way in Gradle-groovy to group related configration in a DSL-like syntax.

Besidest that, even with

dependencyCheck {
   proxy {
      port = 0
   }
}

Gradle doesn't complain, but IntelliJ IDEA (correctly) does, as this is passing a closure.

image

@Vampire
Copy link
Contributor Author

Vampire commented Jan 13, 2025

it wasn't obvious to me that moving to dot syntax is what it takes to move to using the actions overloads.

That would be quite strange and is not what is happening as I said above.
It is another way of doing the configuration which you can use almost anywhere in Gradle.
The "scoped" / "grouped" way ist just more often used as it looks nicer usually.
In some cases the "grouped" way is different, for example as it might be evaluated lazily while dotted syntax is eager,
especially without Propertys being used, but here this is not the case, as the methods would also right away evaluate the given "group".

@chadlwilson
Copy link
Contributor

it wasn't obvious to me that moving to dot syntax is what it takes to move to using the actions overloads.

That would be quite strange and is not what is happening as I said above. It is another way of doing the configuration which you can use almost anywhere in Gradle.

OK, maybe better to ignore whether it is "actions syntax" or "neither" from my comment - I'm probably better to have written it as "to move to using a non-deprecated approach". I don't actually know how Gradle DSL magic translates these syntaxes internally to bind to the overloads/extension config classes, especially when using @CompileStatic (and shouldn't really have to, as a user).

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.

Allow passing Action<T> when configuring nested extensions
3 participants