Issue: #952 Make project.sync() a public API #1137
Conversation
There was a problem hiding this comment.
Thanks @hotchemi.
The changes look good to me, except for the documentation comments and some test coverage. I'd like to see some more test coverage in SyncTaskIntegrationTest like we have in CopyTaskIntegrationSpec.
Specifically, I'd like to see a "sync action" test that's like "copy action" where the build has a set of source files and the destination has some files already in it. The build script should define a task that uses project.sync. After the task runs, the destination should have all files from the source and none of the pre-existing files.
And then I'd like to see the same sort of test for "copy single files", "copy from file tree", "copy from file collection", and "copy from composite file collection", but using project.sync. The important difference with this is that we should have something in the destination before running the task and ensure that it's removed after running the task.
Thanks again for helping out.
| * Synchronizes the contents of a destination directory with some source directories and files. | ||
| * | ||
| * <p> | ||
| * This task is like the {@link #copy(Action)} task, except the destination directory will only contain the files copied. |
There was a problem hiding this comment.
This isn't a "task", so we should probably just say This method is like {@link #copy(Action)}, ...
| @@ -1452,6 +1452,41 @@ | |||
| CopySpec copySpec(); | |||
|
|
|||
| /** | |||
| * Synchronizes the contents of a destination directory with some source directories and files. | |||
There was a problem hiding this comment.
We should make this follow with copy(Action) says, e.g.:
Synchronizes the contents of a destination directory with some source directories and files. The given action is used to configure a {@link CopySpec}, which is then used to synchronize the files.
| * <p> | ||
| * Examples: | ||
| * <pre autoTested=''> | ||
| * // Sync can be used like a Copy task |
There was a problem hiding this comment.
Like copy(Closure), we should have an example that uses project.sync instead of the Sync task.
|
@big-guy Thank you for your polite review and sorry for my misunderstanding🙇 Addressed at a8cda8c. |
|
🙇 |
|
@hotchemi Thanks for your help. Could you follow up on adding some integration tests that I described here: #1137 (review) ? |
|
@big-guy Ah sorry I missed...let me work on. |
|
@hotchemi Are you still planning to add the requested tests? |
|
@bmuschko Ah sorry I forgot to do that...let me work on 🙇 |
- Add Project.java to the signature and doc. - Add the method to the DSL. - Add integration tests for new exposed api.
|
@big-guy Tests were added. Can you have another look? |
|
Thanks @hotchemi, this will be in the next release (after 3.5). |
|
@big-guy Thx 🙇 |
Overview
WorkResult sync(Action<? super CopySpec> action);and doc toProject.java.Any of the checked boxes below indicate that I took action:
./gradlew quickCheckFor all non-trivial changes that modify the behavior or public API:
the forum or can reference a JIRA issue.
brief but explains the use case or problem you are trying to solve,
touches on the planned implementation approach as well as the test cases
that verify the behavior. Optimally, design documents should be submitted
as a separate pull request. Samples
can be found in the Gradle GitHub repository. Please let us know if you need help with
creating the design document. We are happy to help!
test coverage to verify the behavior. Before submitting the pull request
I ran a build on your local machine via the command
./gradlew quickCheck <impacted-subproject>:check.DSL reference and Javadocs where applicable.