Support using task outputs and configurations as config files. #180
Conversation
README.md
Outdated
|
|
||
| cargo { | ||
| containerId = 'jboss5x' | ||
|
|
||
| local { | ||
| configFile { | ||
| file = file('src/main/jboss5/login-config.xml') | ||
| files = file('src/main/jboss5/login-config.xml') |
There was a problem hiding this comment.
Assigning a single file calling a setter on a file collection looks a little bit funny from a users perspective. Can we make it more obvious that it is basically an add operation?
There was a problem hiding this comment.
I'm not happy about this design either but I struggled to find a clean way of supporting file collections while not breaking the existing api. I'm happy to rework if you can suggest something better by saying how do you see the dsl to look when using a single file as a config file source and when using a file collection as a config file source.
Btw, this is not an add operation and actually a setter, so if you say:
cargo {
local {
configFile {
files = file('src/main/jboss5/sample-roles.properties')
files = file('src/main/jboss5/sample-users.properties')
toDir = 'conf/props'
}
}
}
only the second file will be copied.
There was a problem hiding this comment.
I am happy to break the existing API. It doesn't have to be backward compatible.
How about we just make it a setter and you have to provide a list of Files or a list of Strings? I'd remove the existing file method in the DSL and only provide a single way to set the configuration files.
There was a problem hiding this comment.
Ok, I will remove the file method in the DSL and leave the setter for files which will still be backed by Project.files() and take whatever that method takes, so all of the following will work:
files = 'src/main/jboss5/sample-roles.properties'
files = file('src/main/jboss5/sample-roles.properties')
files = ['src/main/jboss5/sample-roles.properties']
files = [file('src/main/jboss5/sample-roles.properties')]
files = configurations.configFiles
files = tasks.writeConfigFile
I will also rename ConfigFile class to ConfigFiles and configFile {} to configFiles {} in the DSL.
Can you please confirm that you're happy with all of the above before I do the necessary work?
There was a problem hiding this comment.
Why don't we require a user to provide a FileCollection instead of trying to turn everything into a FileCollection internally by calling Project.files()? I'd find that much clearer from a user's perspective.
There was a problem hiding this comment.
Can do, that's totally fine by me, I just thought it's more convenient and an established pattern in core Gradle DSLs (e.g. CopySpec) which I wanted to follow but requiring a FileCollection is also fine. I'll assume you are happy with the rest of the proposed changes and will update the PR in the coming days.
There was a problem hiding this comment.
I personally find passing an Object array pretty confusing from a user's perspective. It's unclear what types are acceptable. Once you make the change to the API to only accept a FileCollection we are ready to merge. Thanks!
…e collections to be used when configuring config files.
|
|
|
Thanks! Looking forward to the next release. |
|
Was the the last PR you wanted to get merged? I can release a new version today if you want to have the features. |
|
Yes, that was the last one I planned to submit. Having all of the changes I submitted recently released sometime soon would be nice. |
|
I released a new version. The version should be available on the Plugin Portal and Maven Central later today. |
No description provided.