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

Move detekt-gradle-plugin to be a composite build #4751

Merged
merged 3 commits into from Apr 25, 2022
Merged

Conversation

cortinico
Copy link
Member

@cortinico cortinico commented Apr 23, 2022

Stacked on top of #4748

This converts our project to use Gradle Composite builds.
With this we won't need to depend on external, previously published versions of detekt-gradle-plugin. It will help us prevent and catch earlier a lot of bugs we discover during the release process.

I've also introduced a buildAll top level task that builds everything (all the subprojects and run the build task on the included build).

I've removed the applySelfAnalysisVersion task as that's unnecessary now.

@github-actions github-actions bot added build ci core dependencies Pull requests that update a dependency file labels Apr 23, 2022
@cortinico cortinico added this to the 1.21.0 milestone Apr 23, 2022
@codecov
Copy link

codecov bot commented Apr 23, 2022

Codecov Report

Merging #4751 (c094263) into main (17a62ec) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##               main    #4751   +/-   ##
=========================================
  Coverage     84.69%   84.69%           
  Complexity     3413     3413           
=========================================
  Files           490      490           
  Lines         11232    11232           
  Branches       2065     2065           
=========================================
  Hits           9513     9513           
  Misses          675      675           
  Partials       1044     1044           

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 17a62ec...c094263. Read the comment docs.

@cortinico cortinico added notable changes Marker for notable changes in the changelog and removed dependencies Pull requests that update a dependency file labels Apr 23, 2022
Base automatically changed from nc/gradle-plugin-should-not-depend-on-util to main Apr 23, 2022
@cortinico
Copy link
Member Author

cortinico commented Apr 23, 2022

I'd like to hear @3flex opinion on this before we merge it 👍

@cortinico cortinico requested a review from 3flex Apr 23, 2022
Copy link
Collaborator

@3flex 3flex left a comment

Looks good, there may be a few teething issues but I think this is a win overall!

build.gradle.kts Outdated Show resolved Hide resolved
@@ -9,15 +9,14 @@ plugins {
alias(libs.plugins.pluginPublishing)
}

detekt {
source.from("src/functionalTest/kotlin")
Copy link
Collaborator

@3flex 3flex Apr 24, 2022

Choose a reason for hiding this comment

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

This needs to stay, there's no equivalent config anywhere else in the project to add src/functionalTest to the source checked by the detekt task.

Copy link
Member Author

@cortinico cortinico Apr 24, 2022

Choose a reason for hiding this comment

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

Sadly, this can't stay. The detekt-gradle-plugin build can't use itself.

We would have to use again an older published version of the detekt-gradle-plugin which I would like to avoid frankly (as it's the problem we're trying to address here.

I would rather rely on the runWithArgsFile as it should handle also the src/functionalTest/kotlin folder.

Copy link
Sponsor Member

@chao2zhang chao2zhang Apr 24, 2022

Choose a reason for hiding this comment

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

runWithArgsFile checks the entire codebase, so it should cover all sources under src/functionalTest

Copy link
Collaborator

@3flex 3flex Apr 24, 2022

Choose a reason for hiding this comment

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

The reason I added this block to this build file is running :detekt-gradle-plugin:build doesn't check functionalTest sources without it, because none of the configured Gradle detekt tasks in detekt-gradle-plugin checked it, so CI would fail when I pushed and runWithArgsFile ran, and it became quite frustrating. Running runWithArgsFile locally is just another thing to remember to do before pushing, and it's poor DX.

detekt-gradle-plugin build can't use itself.

True, but we can add id("io.gitlab.arturbosch.detekt") version "1.20.0" to the plugins block in detekt-gradle-plugin. I assume renovate will notify about updates, and updating that version still won't be tied to the release process.

Copy link
Member Author

@cortinico cortinico Apr 25, 2022

Choose a reason for hiding this comment

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

I see, thanks for clarifying. I think it's ok to include the id("io.gitlab.arturbosch.detekt") version "1.20.0" and don't bump it as part of the release process (let's renovate do the job as you suggested).

Copy link
Member Author

@cortinico cortinico Apr 25, 2022

Choose a reason for hiding this comment

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

Are you fine with me adding this in a separate PR?

Copy link
Collaborator

@3flex 3flex Apr 25, 2022

Choose a reason for hiding this comment

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

Are you fine with me adding this in a separate PR?

Yep, LGTM!

@github-actions github-actions bot added dependencies Pull requests that update a dependency file gradle labels Apr 24, 2022
@chao2zhang
Copy link
Sponsor Member

chao2zhang commented Apr 24, 2022

This change may impact the Gradle Enterprise trial progress. As we found that functionalTest does not conform to the incremental buil.
https://ge.detekt.dev/s/vue3arlrp3gvw/timeline?anchor=5cf5p64octxus
image
Let's merge this in as soon as we can.

@vercel
Copy link

vercel bot commented Apr 24, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
detekt Ready (Inspect) Visit Preview Apr 24, 2022 at 9:35PM (UTC)

@cortinico cortinico merged commit 0380716 into main Apr 25, 2022
20 checks passed
@cortinico cortinico deleted the nc/composite-build branch Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build ci dependencies Pull requests that update a dependency file notable changes Marker for notable changes in the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants