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

Replace 'artifacts.clear()' with 'isVisible=false' #625

Merged

Conversation

jjohannes
Copy link
Collaborator

This remove the call to 'configurations["archives"].artifacts.clear()'.

It is no longer necessary if we set the 'consumable' configuration to 'isVisible = false'. Which this commit does.

With this change, we also remove the FLAG_CLEAR_ARTIFACTS and FLAG_SILENT_WARNINGS as these are no longer needed.

Fixes #607

Copy link
Owner

@autonomousapps autonomousapps left a comment

Choose a reason for hiding this comment

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

One small requested change, and thank you!

@@ -8,19 +8,14 @@ import org.gradle.api.Project
@Suppress("DEPRECATION") // forUseAtConfigurationTime()
object Flags {

internal const val FLAG_CLEAR_ARTIFACTS = "dependency.analysis.clear.artifacts"
Copy link
Owner

Choose a reason for hiding this comment

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

Would you mind leaving these flags here? (removing the methods is fine.) I'm thinking I might like to do a check at some point to let folks know about flags that no longer do anything to help them clean up their builds.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I reverted this line.

@jjohannes jjohannes force-pushed the remove-artifacts-clear branch from e511146 to cb99410 Compare March 31, 2022 06:38
@mvegter
Copy link
Contributor

mvegter commented Mar 31, 2022

You will have to fix the AssembleArchivesSpec too, as it is now "failing" on the correct task tree:

Caused by: com.google.common.truth.AssertionErrorWithFacts: unexpected (4): :proj:compileJava, :proj:processResources, :proj:classes, :proj:jar |  
-- | --
  | --- |  
  | expected      : [:proj:assemble] |  
  | but was       : [:proj:compileJava, :proj:processResources, :proj:classes, :proj:jar, :proj:assemble]


This remove the call to 'configurations["archives"].artifacts.clear()'.

It is no longer necessary if we set the 'consumable' configuration
to 'isVisible = false'. Which this commit does.

With this change, we also remove the FLAG_CLEAR_ARTIFACTS and
FLAG_SILENT_WARNINGS as these are no longer needed.
@jjohannes jjohannes force-pushed the remove-artifacts-clear branch from cb99410 to eeeded0 Compare March 31, 2022 10:14
@jjohannes
Copy link
Collaborator Author

Thanks @mvegter I did not see that test. Very good that it exists. Fixed it.

@autonomousapps
Copy link
Owner

I also forgot that that test exists! Now I don't need to bother manually testing this :)

@autonomousapps autonomousapps merged commit 6d2351b into autonomousapps:main Mar 31, 2022
@jjohannes jjohannes deleted the remove-artifacts-clear branch August 30, 2023 09:48
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.

Stop clearing artifacts configuration
3 participants