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.

Sign up for GitHub

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

Jump to bottom

More Variant Details for dependencyInsights #19763

Merged
merged 53 commits into from
Mar 9, 2022
Merged

More Variant Details for dependencyInsights #19763

merged 53 commits into from
Mar 9, 2022

Conversation

octylFractal
Copy link
Member

@octylFractal octylFractal commented Jan 30, 2022 •
edited
Loading

This PR adds additional formatting and new table output for successfully resolved dependencies in dependencyInsights which allows for easier exploration of possible variants to match.

Sample output:
sample output

@octylFractal octylFractal added this to the 7.5 RC1 milestone Jan 30, 2022
@octylFractal octylFractal self-assigned this Jan 30, 2022
tresat
tresat requested changes Jan 31, 2022
View reviewed changes
Copy link
Member

@tresat tresat left a comment

Choose a reason for hiding this comment

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

Gave it a read through and left a bunch of suggestions. I tried to be clear about things I thought should definitely be changed vs. where I was just musing about potential cleanups.

...org/gradle/api/internal/artifacts/ivyservice/resolveengine/graph/builder/ComponentState.java Show resolved Hide resolved
...gradle/api/internal/artifacts/ivyservice/resolveengine/result/ComponentResultSerializer.java Outdated Show resolved Hide resolved
...gradle/api/internal/artifacts/ivyservice/resolveengine/result/ComponentResultSerializer.java Outdated Show resolved Hide resolved
...t/src/main/java/org/gradle/api/internal/artifacts/result/DefaultResolvedComponentResult.java Outdated Show resolved Hide resolved
.../diagnostics/src/main/java/org/gradle/api/tasks/diagnostics/DependencyInsightReportTask.java Show resolved Hide resolved
.../diagnostics/src/main/java/org/gradle/api/tasks/diagnostics/DependencyInsightReportTask.java Outdated Show resolved Hide resolved
.../diagnostics/src/main/java/org/gradle/api/tasks/diagnostics/DependencyInsightReportTask.java Outdated Show resolved Hide resolved
...s/diagnostics/src/main/java/org/gradle/api/tasks/diagnostics/internal/text/TablePrinter.java Outdated Show resolved Hide resolved
...s/diagnostics/src/main/java/org/gradle/api/tasks/diagnostics/internal/text/TablePrinter.java Outdated Show resolved Hide resolved
...s/diagnostics/src/main/java/org/gradle/api/tasks/diagnostics/internal/text/TablePrinter.java Outdated Show resolved Hide resolved
big-guy
big-guy requested changes Feb 3, 2022
View reviewed changes
Copy link
Member

@big-guy big-guy left a comment

Choose a reason for hiding this comment

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

Please kick off the performance tests for this so we can see if we have a problem there.

If we need to, we could split this into the changes you have here and a follow on that adds the "Compatibility" column to the output.

When you go to write some tests to exercise the new output (some of these may already exist):

  • Have a test that shows the output for an external dependency
  • Have a test that shows the output for a project dependency
  • Have a test that shows the output for a dependency with a consumer defined variant (look around for "withVariant" and "ComponentMetadataRules" in dependency-management).

I would not use a Java project for all of these tests since there will be many, many variants.

...t/src/main/java/org/gradle/api/internal/artifacts/result/DefaultResolvedComponentResult.java Show resolved Hide resolved
...gradle/api/internal/artifacts/ivyservice/resolveengine/result/ComponentResultSerializer.java Show resolved Hide resolved
...ts/diagnostics/src/main/java/org/gradle/api/tasks/diagnostics/internal/text/StyledTable.java Show resolved Hide resolved
.../diagnostics/src/main/java/org/gradle/api/tasks/diagnostics/DependencyInsightReportTask.java Outdated Show resolved Hide resolved
.../diagnostics/src/main/java/org/gradle/api/tasks/diagnostics/DependencyInsightReportTask.java Outdated Show resolved Hide resolved
.../diagnostics/src/main/java/org/gradle/api/tasks/diagnostics/DependencyInsightReportTask.java Outdated Show resolved Hide resolved
.../diagnostics/src/main/java/org/gradle/api/tasks/diagnostics/DependencyInsightReportTask.java Outdated Show resolved Hide resolved
@octylFractal
Copy link
Member Author

Updated with current output.

octylFractal and others added 16 commits February 10, 2022 14:44
big-guy
big-guy requested changes Feb 15, 2022
View reviewed changes
...gnostics/src/main/java/org/gradle/api/tasks/diagnostics/internal/dependencies/MatchType.java Outdated Show resolved Hide resolved
.../diagnostics/src/main/java/org/gradle/api/tasks/diagnostics/DependencyInsightReportTask.java Outdated Show resolved Hide resolved
.../diagnostics/src/main/java/org/gradle/api/tasks/diagnostics/DependencyInsightReportTask.java Show resolved Hide resolved
.../diagnostics/src/main/java/org/gradle/api/tasks/diagnostics/DependencyInsightReportTask.java Show resolved Hide resolved
.../diagnostics/src/main/java/org/gradle/api/tasks/diagnostics/DependencyInsightReportTask.java Show resolved Hide resolved
.../diagnostics/src/main/java/org/gradle/api/tasks/diagnostics/DependencyInsightReportTask.java Outdated Show resolved Hide resolved
.../diagnostics/src/main/java/org/gradle/api/tasks/diagnostics/DependencyInsightReportTask.java Outdated Show resolved Hide resolved
.../diagnostics/src/main/java/org/gradle/api/tasks/diagnostics/DependencyInsightReportTask.java Show resolved Hide resolved
@octylFractal octylFractal force-pushed the ot/feature/dependency-insight-variants branch from 5c524b1 to 541fad0 Compare February 21, 2022 22:38
@gradle gradle deleted a comment from octylFractal Feb 22, 2022
@DPUkyle
Fix more snippets related to new dependencyInsight report
876f476 
 - `$ ./gradlew :docs:docsTest --tests "*.snippet-dependency-management-inspecting-dependencies*" --no-configuration-cache`
@gradle gradle deleted a comment from octylFractal Mar 3, 2022
@gradle gradle deleted a comment from octylFractal Mar 3, 2022
@octylFractal octylFractal force-pushed the ot/feature/dependency-insight-variants branch from a9f09e0 to ee6c8bb Compare March 3, 2022 21:11
@gradle gradle deleted a comment from DPUkyle Mar 3, 2022
@DPUkyle DPUkyle force-pushed the ot/feature/dependency-insight-variants branch from bfe458e to 5c1a337 Compare March 3, 2022 22:35
@gradle gradle deleted a comment from octylFractal Mar 3, 2022
@gradle gradle deleted a comment from DPUkyle Mar 7, 2022
@octylFractal octylFractal marked this pull request as ready for review March 7, 2022 21:19
@gradle gradle deleted a comment from DPUkyle Mar 7, 2022
@eskatos
Copy link
Member

eskatos commented Mar 8, 2022

@octylFractal good changes in DependencyManagementResultsAsInputsIntegrationTest 👍

@octylFractal octylFractal mentioned this pull request Mar 8, 2022
Merged
@octylFractal
Introduce flag to request all variants (#20118)
05206bb 
* Introduce flag to request all variants

* Document when all variants flag works

* Cleanup test to remove unneeded projects
@octylFractal
Move test comments to when labels
7eadc33
@octylFractal
Fix doc normalizing for JDK 17
775c852
@octylFractal
Copy link
Member Author

@bot-gradle test this

@gradle gradle deleted a comment from octylFractal Mar 9, 2022
@bot-gradle
Copy link
Collaborator

OK, I've already triggered the following builds for you:

DPUkyle
DPUkyle approved these changes Mar 9, 2022
View reviewed changes
Copy link
Contributor

@DPUkyle DPUkyle left a comment

Choose a reason for hiding this comment

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

LGTM pending an answer to my two questions about unchecked casts.

...va/org/gradle/api/tasks/diagnostics/internal/graph/nodes/AbstractRenderableModuleResult.java Show resolved Hide resolved
.../main/java/org/gradle/api/tasks/diagnostics/internal/graph/nodes/ResolvedDependencyEdge.java Show resolved Hide resolved
.../main/groovy/org/gradle/integtests/fixtures/logging/DependencyInsightOutputNormalizer.groovy Show resolved Hide resolved
@octylFractal
Copy link
Member Author

@bot-gradle test and merge

@gradle gradle deleted a comment from octylFractal Mar 9, 2022
@bot-gradle
Copy link
Collaborator

Your PR is queued. See the queue page for details.

@bot-gradle
Copy link
Collaborator

OK, I've already triggered a build for you.

@bot-gradle bot-gradle merged commit f094e1f into master Mar 9, 2022
@octylFractal octylFractal deleted the ot/feature/dependency-insight-variants branch March 9, 2022 20:43
@ljacomet ljacomet mentioned this pull request Jun 15, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eskatos eskatos eskatos requested changes

@tresat tresat tresat approved these changes

@DPUkyle DPUkyle DPUkyle approved these changes

@big-guy big-guy Awaiting requested review from big-guy

Assignees

@octylFractal octylFractal

Labels
None yet
Projects
None yet
Milestone
7.5 RC1
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@octylFractal @eskatos @bot-gradle @tresat @DPUkyle @big-guy