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

If a variant cannot be selected because there are no variants, mentio… #28971

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

mathjeff
Copy link
Contributor

…n that

instead of saying that every variant has no attributes

Fixes: #28970

Context

Contributor Checklist

  • Review Contribution Guidelines.
  • Make sure that all commits are signed off to indicate that you agree to the terms of Developer Certificate of Origin.
  • Make sure all contributed code can be distributed under the terms of the Apache License 2.0, e.g. the code was written by yourself or the original code is licensed under a license compatible to Apache License 2.0.
  • Check "Allow edit from maintainers" option in pull request so that additional changes can be pushed by Gradle team.
  • Provide integration tests (under <subproject>/src/integTest) to verify changes from a user perspective.
  • Provide unit tests (under <subproject>/src/test) to verify logic.
  • Update User Guide, DSL Reference, and Javadoc for public-facing changes.
  • Ensure that tests pass sanity check: ./gradlew sanityCheck.
  • Ensure that tests pass locally: ./gradlew <changed-subproject>:quickTest.

Reviewing cheatsheet

Before merging the PR, comments starting with

  • ❌ ❓must be fixed
  • 🤔 💅 should be fixed
  • 💭 may be fixed
  • 🎉 celebrate happy things

…n that

instead of saying that every variant has no attributes

Fixes: gradle#28970

Signed-off-by: Jeff Gaston <gastoj3@gmail.com>
@mathjeff mathjeff requested a review from a team as a code owner April 25, 2024 21:08
@mathjeff mathjeff requested a review from tresat April 25, 2024 21:08
@bot-gradle bot-gradle added from:contributor PR by an external contributor to-triage labels Apr 25, 2024
@ov7a ov7a added in:dependency-resolution engine metadata 👋 team-triage Issues that need to be triaged by a specific team and removed to-triage labels Apr 29, 2024
@ov7a
Copy link
Member

ov7a commented Apr 29, 2024

Thank you for your contribution!

This PR has a valid DCO and tests. The relevant team for this area will confirm the design of the implementation choices.

@big-guy big-guy added this to the 8.9 RC1 milestone May 6, 2024
@big-guy big-guy removed the 👋 team-triage Issues that need to be triaged by a specific team label May 6, 2024
@big-guy
Copy link
Member

big-guy commented May 6, 2024

@tresat will take a look

- Clarify error message.
- Use an IncompatibleGraphVariantFailure instead of ConfigurationNotFoundFailure, which fixes tests to pass by causing proper FailureDescriber to fire.
- Add no variants exist failure demonstration to error showcase.
Copy link

gitstream-cm bot commented May 7, 2024

⚠️ 'There are new TODOs present in this change. Should any be removed?'

@tresat tresat added the re:comprehensibility reasonable errors and warnings, clear dsl, mental overload label May 7, 2024
@tresat
Copy link
Member

tresat commented May 7, 2024

Thanks for submitting this @mathjeff! I think this is a good clarifying change to a possibly very confusing scenario.

I made some adjustments to the way the failure is created that were necessary to make the test you submitted pass, and made some other minor changes. Assuming CI doesn't find any issues, I'll merge this for Gradle 8.9.

@mathjeff
Copy link
Contributor Author

mathjeff commented May 8, 2024

Thanks!

- This new message clarifies the situation in each of these cases.
@mathjeff mathjeff requested review from a team as code owners May 8, 2024 12:43
@gradle gradle deleted a comment from tresat May 8, 2024
@tresat
Copy link
Member

tresat commented May 8, 2024

@bot-gradle test and merge

@gradle gradle deleted a comment from tresat May 8, 2024
@bot-gradle bot-gradle added this pull request to the merge queue May 8, 2024
@tresat tresat added the a:feature A new functionality label May 8, 2024
@bot-gradle
Copy link
Collaborator

The merge queue build has failed. Click here to see all failures.

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 8, 2024
@tresat
Copy link
Member

tresat commented May 8, 2024

@bot-gradle test and merge

@gradle gradle deleted a comment from tresat May 8, 2024
@bot-gradle bot-gradle added this pull request to the merge queue May 8, 2024
@bot-gradle
Copy link
Collaborator

WARN: Based on labels, this pull request addresses notable issue but no changes to release note found.

Copy link

gitstream-cm bot commented May 8, 2024

📊 Changes by Platform: this PR is 88% new code

3 platforms were affected

See details
Platform Added Lines % of Total Line Changes Deleted Lines % of Total Line Changes Files Changed % of Total Files Changed
null 10 6% 4 2% 2 25%
software 143 81% 16 9% 5 63%
native 2 1% 1 1% 1 13%

@bot-gradle
Copy link
Collaborator

The merge queue build has failed. Click here to see all failures.

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 min review 10 min review a:feature A new functionality from:contributor PR by an external contributor in:dependency-resolution engine metadata ⚠️ Includes TODOs re:comprehensibility reasonable errors and warnings, clear dsl, mental overload
Projects
None yet
Development

Successfully merging this pull request may close these issues.

If dependency project has no variants, mention that there are none
5 participants