-
Notifications
You must be signed in to change notification settings - Fork 264
Improve error message for failed imports #298
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
Improve error message for failed imports #298
Conversation
Signed-off-by: Arve Knudsen <[email protected]>
Signed-off-by: Arve Knudsen <[email protected]>
As mentioned in slack, adding the mage build tag would change the behavior for folks currently using mage, and that could break stuff. You don't need the mage tag on stuff you import. As you pointed out in slack, the comment on this in the docs is not clear at all, and I welcome proposed changes to the wording. I do like the fix to print a better error message, however. |
Signed-off-by: Arve Knudsen <[email protected]>
Signed-off-by: Arve Knudsen <[email protected]>
@natefinch Thanks, I modified the PR to only improve the error message (along with a test). Please have another look. |
Signed-off-by: Arve Knudsen <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, Thanks!
Signed-off-by: Arve Knudsen <[email protected]>
Signed-off-by: Arve Knudsen <[email protected]>
Signed-off-by: Arve Knudsen <[email protected]>
Signed-off-by: Arve Knudsen <[email protected]>
Signed-off-by: Arve Knudsen <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thank you!
This PR improves the error message when importing packages fails, for example because an imported package has a build tag (e.g.
mage
). A test is also included.Plus, I edited the documentation to clarify how Mage doesn't support importing package files with build tags.