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

Add Sample rule that copies a sample to a directory for testing #6

Merged
merged 3 commits into from
Jul 9, 2018

Conversation

marcphilipp
Copy link
Contributor

This PR extracts a JUnit 4 TestRule from the Sample rule present in gradle/gradle and makes it reusable without relying on any Gradle specific classes.

This commit extracts a JUnit 4 TestRule from the Sample rule present
in gradle/gradle and makes it reusable without relying on any Gradle
specific classes.
@marcphilipp marcphilipp requested review from eriwen and wolfs June 14, 2018 09:45
defaultSampleName);
}

private static SourceSampleDirSupplier eagerSourceSampleDirSupplier(final String sourceBaseDirPath) {
Copy link
Member

Choose a reason for hiding this comment

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

Eager doesn't seem to be the right name. How about rootDirSourceSampleDirProvider?

this(sourceBaseDirPath, temporaryFolder, null);
}

public Sample(final String sourceBaseDirPath, final TemporaryFolder temporaryFolder, @Nullable String defaultSampleName) {
Copy link
Member

Choose a reason for hiding this comment

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

How about we don't expose a long/many constructors but some builder like methods instead? This way we can keep the API stable/extendable and the class is easier to use.

@marcphilipp
Copy link
Contributor Author

@wolfs PTAL!

@marcphilipp
Copy link
Contributor Author

@eriwen Would you like to take a look and potentially merge this PR?

@eriwen eriwen self-assigned this Jul 9, 2018
@eriwen eriwen merged commit a0e2305 into master Jul 9, 2018
@eriwen eriwen deleted the marc/sample-rule branch July 9, 2018 23:49
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.

None yet

3 participants