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

Support dependency on taskProvider (among other things) #2946

Merged
merged 1 commit into from
Jul 3, 2024

Conversation

oldergod
Copy link
Member

@oldergod oldergod commented May 16, 2024

fixes #2933

@oldergod oldergod force-pushed the bquenaudon.2024-05-09.srcDirTaskProvider branch 5 times, most recently from 244173f to 9beef31 Compare July 3, 2024 14:59
@oldergod oldergod requested review from martinbonnin, staktrace, Egorand, JakeWharton and swankjesse and removed request for staktrace July 3, 2024 15:02
@oldergod oldergod changed the title Support dependency on taskProvider Support dependency on taskProvider (among other things) Jul 3, 2024
Copy link
Collaborator

@staktrace staktrace left a comment

Choose a reason for hiding this comment

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

Seems reasonable enough. I'm concerned about the change in ABI, though. There could be gradle plugins that are compiled against the old version of wire (with srcDir(String)) which will break if used in a build with the new version of wire on the buildscript classpath. It might be better to lave srcDir alone and add a new function that takes Any so that there's no ABI breakage

@oldergod
Copy link
Member Author

oldergod commented Jul 3, 2024

I can keep srcDir(string) alongside it

@oldergod oldergod force-pushed the bquenaudon.2024-05-09.srcDirTaskProvider branch from 9beef31 to d3bef4d Compare July 3, 2024 15:58
@oldergod oldergod marked this pull request as ready for review July 3, 2024 15:59
Copy link
Collaborator

@staktrace staktrace left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM

/** Sets a directory. Example: "src/main/proto". */
@Deprecated("Deprecated in favor of 'srcDir(fileCollection)'")
Copy link
Member

Choose a reason for hiding this comment

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

If you are deprecating this function, seems like the one below it should go as well.

Copy link
Collaborator

@martinbonnin martinbonnin left a comment

Choose a reason for hiding this comment

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

🚀

@@ -291,11 +293,25 @@ open class WireExtension(
/** Calling this will resolve the configuration. */
internal val roots: Set<File>
get() = configuration.files + sourceDirectoriesAndLocalJars
private val files: ConfigurableFileCollection by lazy(NONE) {
val files = project.files()
project.dependencies.add(configuration.name, files)
Copy link
Collaborator

Choose a reason for hiding this comment

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

TIL you could pass a FileCollection as a dependency

@oldergod oldergod force-pushed the bquenaudon.2024-05-09.srcDirTaskProvider branch from d3bef4d to e82f6bb Compare July 3, 2024 20:29
@oldergod oldergod merged commit 01fee77 into master Jul 3, 2024
11 checks passed
@oldergod oldergod deleted the bquenaudon.2024-05-09.srcDirTaskProvider branch July 3, 2024 21:35
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.

srcDir should accept TaskProvider
4 participants