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

Clarify dependency violation message #423

Merged
merged 1 commit into from
Jan 23, 2025

Conversation

derekprior
Copy link
Contributor

What are you trying to accomplish?

I was bothered by a couple of things in the dependency violation message and this change fixes them.

  1. "Are we missing an abstraction?" is not helpful context.
  2. "Is the code making the reference, and the referenced constant, in the right packages?" was not grammatically correct or clear.

What approach did you choose and why?

I edited the copy and updated the tests.

What should reviewers focus on?

Whether you think this error message is more clear or helpful

Type of Change

  • Bugfix
  • New feature
  • Non-breaking change (a change that doesn't alter functionality - i.e., code refactor, configs, etc.)

Additional Release Notes

  • Breaking change (fix or feature that would cause existing functionality to change)

Include any notes here to include in the release description. For example, if you selected "breaking change" above, leave notes on how users can transition to this version.

If no additional notes are necessary, delete this section or leave it unchanged.

Checklist

  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • It is safe to rollback this change.

@derekprior derekprior requested a review from a team as a code owner January 17, 2025 22:02
I was bothered by a couple of things in the dependency violation message
and this change fixes them.

1. "Are we missing an abstraction?" is not helpful context.
2. "Is the code making the reference, and the referenced constant, in
   the right packages?" was not grammatically correct or clear.
@derekprior derekprior force-pushed the dp-dependency-violation-messages branch from 8fa0500 to bedddc2 Compare January 17, 2025 22:06
Copy link
Member

@gmcgibbon gmcgibbon left a comment

Choose a reason for hiding this comment

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

This seems clearer to me. Thank you

@gmcgibbon gmcgibbon merged commit 3c6b354 into Shopify:main Jan 23, 2025
9 checks passed
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.

2 participants