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

Check jcenter repository present in Gradle plugin #2550

Merged
merged 8 commits into from
Mar 29, 2020

Conversation

rock3r
Copy link
Contributor

@rock3r rock3r commented Mar 29, 2020

This PR fixes #2549 by adding a log when the user doesn't have jcenter() configured in their project, by emitting an error with a clear warning and resolution instructions.

I have run all the tests in the project and smoke tested the plugin locally.

Whenever the plugin is applied to a project that doesn't have
jcenter() configured, the user would get a cryptic error message
from Gradle (see issue detekt#2549). This simple change makes sure the
plugin will provide users with clear instructions on how to fix
their build so they don't get stumped when things don't work.
Also update repo URL to /pinterest/ktlint through out docs
This is necessary because we need to make sure it's there _somehow_,
and it may be added by other plugins, or blocks evaluated after this
plugin is applied.
@codecov-io
Copy link

codecov-io commented Mar 29, 2020

Codecov Report

Merging #2550 into master will increase coverage by 1.68%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2550      +/-   ##
============================================
+ Coverage     25.20%   26.89%   +1.68%     
+ Complexity      392      390       -2     
============================================
  Files           376      377       +1     
  Lines          7355     6671     -684     
  Branches       1206     1207       +1     
============================================
- Hits           1854     1794      -60     
+ Misses         5370     4746     -624     
  Partials        131      131              
Impacted Files Coverage Δ Complexity Δ
...arturbosch/detekt/formatting/FormattingProvider.kt 0.00% <ø> (ø) 0.00 <0.00> (ø)
...kotlin/io/gitlab/arturbosch/detekt/DetektPlugin.kt 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
.../arturbosch/detekt/internal/GradleProjectChecks.kt 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...ab/arturbosch/detekt/rules/style/WildcardImport.kt 46.15% <ø> (+6.15%) 1.00 <0.00> (ø)
...itlab/arturbosch/detekt/api/internal/Validation.kt 33.33% <0.00%> (-16.67%) 0.00% <0.00%> (ø%)
.../gitlab/arturbosch/detekt/cli/console/Colorizer.kt 61.53% <0.00%> (-9.90%) 0.00% <0.00%> (ø%)
...h/detekt/api/internal/DisabledAutoCorrectConfig.kt 55.55% <0.00%> (-8.09%) 4.00% <0.00%> (ø%)
...otlin/io/gitlab/arturbosch/detekt/core/TaskPool.kt 33.33% <0.00%> (-6.67%) 3.00% <0.00%> (ø%)
...in/io/gitlab/arturbosch/detekt/api/SingleAssign.kt 60.00% <0.00%> (-6.67%) 3.00% <0.00%> (ø%)
...in/kotlin/io/gitlab/arturbosch/detekt/api/Issue.kt 50.00% <0.00%> (-5.56%) 2.00% <0.00%> (ø%)
... and 261 more

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 8a48551...72854a4. Read the comment docs.

Copy link
Member

@schalkms schalkms left a comment

Choose a reason for hiding this comment

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

Thanks for improving the docs as well as investigating and fixing the jcenter issue with a better error description! Lgtm!

Co-Authored-By: M Schalk <30376729+schalkms@users.noreply.github.com>
@arturbosch arturbosch added this to the 1.8.0 milestone Mar 29, 2020
@arturbosch arturbosch merged commit c2a794a into detekt:master Mar 29, 2020
@mtraynham
Copy link

mtraynham commented Apr 2, 2020

Is there any way to disable this? Currently using Nexus with a proxy jcenter repository.

@rock3r
Copy link
Contributor Author

rock3r commented Apr 2, 2020

No, but it's just a log message. It doesn't stop the build in and by itself.

@rock3r rock3r deleted the check_jcenter_repository_present branch April 2, 2020 06:12
@BraisGabin
Copy link
Member

🤔 I know nearly nothing about gradle plugins but... Could we launch this message only if it fails? As a hint about what could be happening?

@rock3r
Copy link
Contributor Author

rock3r commented Apr 2, 2020

To be able to do that you'd have to check if the dependency is available on the classpath of the modules after the project is synced... possibly doable but a bit beyond my Gradle knowledge :)

@runningcode
Copy link
Contributor

We use a maven proxy so this disallows us from upgrading. I'll file a new issue.

@rock3r
Copy link
Contributor Author

rock3r commented Apr 2, 2020

Again, the plugin will work just fine, it's just emitting a log entry that you can ignore.

@runningcode
Copy link
Contributor

I understand, but in our app with over 100 modules its printed to the console over 100 times as an "error". Let's keep the discussion going on this issue: #2571 (comment)

@rock3r rock3r mentioned this pull request Apr 2, 2020
arturbosch added a commit that referenced this pull request Apr 3, 2020
This pull request was closed.
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.

Adding Detekt to a project from scratch, detektGenerateConfig is broken
7 participants