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

Improve provide conflict error message #13661

Merged
merged 2 commits into from
Apr 10, 2023
Merged

Improve provide conflict error message #13661

merged 2 commits into from
Apr 10, 2023

Conversation

uilianries
Copy link
Member

@uilianries uilianries commented Apr 10, 2023 •
edited by czoido
Loading

Changelog: Fix: Improve provides conflict message error.
Docs: Omit

  • Split up old GraphError generic class to specialized classes
  • No more consult .kind attribute, each class has its own information
  • Update tests which depends on graph error
  • Improve graph error messages.

fixes #13548

  • Refer to the issue that supports this Pull Request.
  • If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
  • I've read the Contributing guide.
  • I've followed the PEP8 style guides for Python code.
  • I've opened another PR in the Conan docs repo to the develop branch, documenting this one.

Note: By default this PR will skip the slower tests and will use a limited set of python versions. Check here how to increase the testing level by writing some tags in the current PR body text.

memsharded
memsharded reviewed Apr 10, 2023
View reviewed changes
Copy link
Member

@memsharded memsharded left a comment

Choose a reason for hiding this comment

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

I am good with the refactor of dedicated classes for each error type, please see the minor comments.

conans/client/graph/error/conflict_error.py Outdated Show resolved Hide resolved
conans/client/graph/error/loop_error.py Outdated Show resolved Hide resolved
conans/client/graph/graph_builder.py Outdated Show resolved Hide resolved
@uilianries uilianries changed the base branch from develop2 to release/2.0 April 10, 2023 14:03
@uilianries uilianries changed the base branch from release/2.0 to develop2 April 10, 2023 14:05
@uilianries
Refactor GraphError
48260cf 
- Split each class part to a separated class
- Improve output messages for each error
- Update tests to use the new classes
- Update exception catch to use dedicated classes

Signed-off-by: Uilian Ries <uilianries@gmail.com>
@uilianries uilianries changed the base branch from develop2 to release/2.0 April 10, 2023 14:16
memsharded
memsharded approved these changes Apr 10, 2023
View reviewed changes
Copy link
Member

@memsharded memsharded left a comment

Choose a reason for hiding this comment

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

Looks good!

@uilianries uilianries marked this pull request as ready for review April 10, 2023 14:18
@uilianries
Fix super class
804c57e 
Signed-off-by: Uilian Ries <uilianries@gmail.com>
AbrilRBS
AbrilRBS approved these changes Apr 10, 2023
View reviewed changes
Copy link
Member

@AbrilRBS AbrilRBS left a comment

Choose a reason for hiding this comment

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

Nice!

@memsharded memsharded added this to the 2.0.4 milestone Apr 10, 2023
@memsharded memsharded merged commit 1e9b8b5 into conan-io:release/2.0 Apr 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@memsharded memsharded memsharded approved these changes

@AbrilRBS AbrilRBS AbrilRBS approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2.0.4
Development

Successfully merging this pull request may close these issues.

[bug] provide conflict is not informative
3 participants
@uilianries @AbrilRBS @memsharded