Skip to content

Tweak GHA configs a bit - #5852

Merged
BraisGabin merged 8 commits into
detekt:mainfrom
Goooler:tweak_ci
Mar 2, 2023
Merged

Tweak GHA configs a bit#5852
BraisGabin merged 8 commits into
detekt:mainfrom
Goooler:tweak_ci

Conversation

Comment thread .github/workflows/pre-merge.yaml
# Conflicts:
#	.github/workflows/pre-merge.yaml
- name: Build Detekt Documentation
uses: gradle/gradle-build-action@6095a76664413da4c8c134ee32e8a8ae900f0f1f # v2
with:
gradle-home-cache-cleanup: true

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not sure why this is not their default value.

If in one action we execute just help and in other build. We'll the help action remove a lot of files that are used by the build one? My point is, do we know if this is always a net win? Does it have side effects?

I assume that the desired side effect is that if we would like to roll back a gradle upgrade we would need to download some things again. But that is completely reasonable and unlikely to happen. But are there others?

My fear is that this would create far too many misses.

@Goooler Goooler Feb 28, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I enable this flag on our internal repos, cause the cache sizes grow up to about 2~3 GB, spend too much time on restoring caches. Either way, I can revert these lines if you want.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

My fear is that this would create far too many misses.

Let's merge it for now, and report back to Gradle if this degrading the GE experience (it's their custom GH Action afterall)

- name: Build Detekt Documentation
uses: gradle/gradle-build-action@6095a76664413da4c8c134ee32e8a8ae900f0f1f # v2
with:
gradle-home-cache-cleanup: true

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

My fear is that this would create far too many misses.

Let's merge it for now, and report back to Gradle if this degrading the GE experience (it's their custom GH Action afterall)

# Conflicts:
#	.github/workflows/detekt-with-type-resolution.yaml
@github-actions

github-actions Bot commented Mar 2, 2023

Copy link
Copy Markdown
Warnings
⚠️ This PR is approved with no milestone set. If merged, it won't appear in the Detekt release notes.

Generated by 🚫 dangerJS against ea65fb8

@codecov

codecov Bot commented Mar 2, 2023

Copy link
Copy Markdown

Codecov Report

Merging #5852 (ea65fb8) into main (5714878) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##               main    #5852   +/-   ##
=========================================
  Coverage     84.57%   84.57%           
  Complexity     3785     3785           
=========================================
  Files           546      546           
  Lines         12944    12944           
  Branches       2274     2274           
=========================================
  Hits          10948    10948           
  Misses          862      862           
  Partials       1134     1134           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@BraisGabin
BraisGabin merged commit 0989b60 into detekt:main Mar 2, 2023
@BraisGabin BraisGabin added this to the 1.23.0 milestone Mar 2, 2023
@BraisGabin BraisGabin added the housekeeping Marker for housekeeping tasks and refactorings label Mar 2, 2023
@BraisGabin

Copy link
Copy Markdown
Member

Thanks for the cleanup :)

@Goooler
Goooler deleted the tweak_ci branch March 2, 2023 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci housekeeping Marker for housekeeping tasks and refactorings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants