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

Use SourceSet in task names and reports files names. #180

Merged
merged 1 commit into from
Dec 6, 2018

Conversation

Tapchicoma
Copy link
Collaborator

@Tapchicoma Tapchicoma commented Nov 29, 2018

After reflecting on last discussion in #153, I think that it is better to rename check/format tasks, so they include SourceSet in their name (generated reports names are adjusted as well).

This better reflects on what exactly sources task are working.

Related to #170

@JLLeitschuh
Copy link
Owner

This is an API breaking change is it not?

@Tapchicoma
Copy link
Collaborator Author

Kind of, though I don't think it is something very breaking:

  • hidden tasks names changed from ktlintMainCheck to ktlintMainSourceSetCheck
  • generated report names also changed accordingly

Meta tasks names stays the same.

@JLLeitschuh
Copy link
Owner

Yea, I still think we have to mark this as a breaking change, unfortunately.

@Tapchicoma
Copy link
Collaborator Author

Fine for me, we can do next release as 7.0.0. I am also considering to use Gradle 5.0 as minimal supported version.

@JLLeitschuh
Copy link
Owner

Sure, that would make sense.

@Tapchicoma
Copy link
Collaborator Author

@JLLeitschuh updated changes in CHANGELOG.md to have better phrasing and include breaking word

This better reflects on what exactly sources this tasks are working.

Signed-off-by: Yahor Berdnikau <egorr.berd@gmail.com>
/**
* Create check task name from source set name.
*/
internal fun String.sourceSetCheckTaskName() = "ktlint${capitalize()}SourceSetCheck"
Copy link
Contributor

Choose a reason for hiding this comment

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

It is so nice with Kotlin to be able to do this.

@tasomaniac
Copy link
Contributor

I don't think there is such think as hidden tasks. All the tasks created are public. If they are not put into a group then they are less visible. Since these tasks are grouped in verification, I would say they are pretty public. 👍 for making it a breaking change.

@@ -10,6 +10,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- ?
### Changed
- Update Kotlin to `1.3.10` version
- Breaking: check/format tasks for specific source sets
Copy link
Contributor

@tasomaniac tasomaniac Dec 4, 2018

Choose a reason for hiding this comment

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

Would be great to have an example for Android. Something like below?

Example: In an Android project with foo flavor, ktlintFooDebugSourceSetCheck task will check the foo sourceSet (not main). ktlintFooDebugCheck meta task will check all the sourceSets for fooDebug build variant.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I will add this explanation to the next PR that adds such variant meta tasks, as it, imho, belongs there.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed 👍

@JLLeitschuh
Copy link
Owner

I don't have any additional feedback to offer. If you want to add an example like @tasomaniac suggested, go for it. Otherwise, I'm happy to let this in as is.

@Tapchicoma Tapchicoma merged commit f40340a into JLLeitschuh:master Dec 6, 2018
@Tapchicoma Tapchicoma deleted the adjust-task-names branch December 6, 2018 19:58
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