Skip to content
This repository has been archived by the owner on Feb 11, 2022. It is now read-only.

Fix ktlint for all versions #153

Merged
merged 5 commits into from Dec 17, 2018
Merged

Conversation

tasomaniac
Copy link
Contributor

@tasomaniac tasomaniac commented Dec 3, 2018

Ktlint integration is fixed for 6.x and upcoming 7.x versions.

For Ktlint integration, we only depend on task names and a certain property (reportOutputFiles). Extracted task matching into a function and support resolving the tasks from all versions of ktlint.

Tests

6.1.0 is also fixed but has a bug affecting our test setup. I don't think it is worth to fix that only for tests. 6.2.x and 6.3.x are supported.

Also enabled tests for 6.1.0 version by allowing test fixtures to be copied into the actual test project.

@tasomaniac tasomaniac changed the base branch from master to develop December 3, 2018 22:24
@tasomaniac
Copy link
Contributor Author

I was thinking before that this should be fixed in ktlint. But having a second round of thinking, I found an easy way to support both behaviors of the ktlint plugin.

Copy link
Contributor

@mr-archano mr-archano left a comment

Choose a reason for hiding this comment

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

Left a couple of questions, as I don't understand what's going on exactly. I will try to play with this branch myself and understand more.

Copy link
Contributor Author

@tasomaniac tasomaniac left a comment

Choose a reason for hiding this comment

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

Let's not merge this just now. Ktlint plugin is being updated again and will break backward compatibility again. I think I can still fix it.

But going forward, we should really think about implementing our own. It is not super easy but also not super hard.

@tasomaniac
Copy link
Contributor Author

Hey @mr-archano, did some changes to support the upcoming ktlint changes. I think this is ready to review and merge. Once they publish the plugin, I will add new tests for 7.x versions.

Also had a small addition in test to enable the commented out test.

@@ -63,6 +63,13 @@ ${project.additionalConfiguration}
file.text = text
}

public T copyIntoSourceSet(String sourceSet, File srcDir) {
Copy link
Contributor

Choose a reason for hiding this comment

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

💯

@tasomaniac
Copy link
Contributor Author

@hal90002 retest this please

@tasomaniac tasomaniac merged commit 27c99b3 into develop Dec 17, 2018
@tasomaniac tasomaniac deleted the taso/fix-ktlint-all-versions branch December 17, 2018 22:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants