Navigation Menu

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

Improve handling of classpaths in Gradle plugin #1609

Merged
merged 4 commits into from May 18, 2019
Merged

Conversation

3flex
Copy link
Member

@3flex 3flex commented Apr 23, 2019

This change achieves a few things:

This change should not impact anyone's existing build configuration.

cc: @marschwar

@3flex 3flex force-pushed the 1208-fix branch 2 times, most recently from 2005719 to f6d61bc Compare April 23, 2019 13:46
@marschwar
Copy link
Contributor

The code looks nice and clean to me.

Is there a use case for users of the detekt plugin to set the detektClasspath or is that mainly to be able to build the plugin against the latest cli?
I am just starting to get a feel for the dependency management with the gradle plugin, so please excuse my ignorance.

@3flex
Copy link
Member Author

3flex commented Apr 25, 2019

Is there a use case for users of the detekt plugin to set the detektClasspath

Usually, no, I expect users would just use the defaults and possibly change the version of the CLI dependency using toolVersion. It's more important for the task so it does proper up-to-date checking.

@3flex 3flex force-pushed the 1208-fix branch 2 times, most recently from 43288dc to b7f5564 Compare May 1, 2019 11:13
@codecov-io
Copy link

Codecov Report

Merging #1609 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1609   +/-   ##
=========================================
  Coverage     79.62%   79.62%           
  Complexity     1963     1963           
=========================================
  Files           335      335           
  Lines          5527     5527           
  Branches       1014     1014           
=========================================
  Hits           4401     4401           
  Misses          593      593           
  Partials        533      533

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5e64f21...b7f5564. Read the comment docs.

@codecov-io
Copy link

codecov-io commented May 1, 2019

Codecov Report

Merging #1609 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##             master   #1609   +/-   ##
========================================
  Coverage      76.2%   76.2%           
  Complexity     1841    1841           
========================================
  Files           318     318           
  Lines          5456    5456           
  Branches       1007    1007           
========================================
  Hits           4158    4158           
  Misses          754     754           
  Partials        544     544

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 54a1187...eb6a04c. Read the comment docs.

Copy link
Member

@arturbosch arturbosch left a comment

Choose a reason for hiding this comment

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

This looks finish to me, this PR just needs CI restart? Correct me if I'm wrong.

@3flex
Copy link
Member Author

3flex commented May 15, 2019

Now has merge conflicts, I can rebase so will rerun CI.

@arturbosch
Copy link
Member

Yes please, sorry for the trouble.

@3flex 3flex merged commit 45dc3c0 into detekt:master May 18, 2019
@3flex 3flex deleted the 1208-fix branch May 18, 2019 01:07
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.

Project dependency to override detekt-cli in composite build does not run assemble automatically
4 participants