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
fix(pkg): improve locations of conversion errors #8828
fix(pkg): improve locations of conversion errors #8828
Conversation
9b7b705
to
3dd00a1
Compare
Can you add a test so we can see what it looks like? |
6cadfac
to
6d0941f
Compare
6d0941f
to
416475a
Compare
We don't have the exact locations, but we can at least point to the relevant files. Signed-off-by: Rudi Grinberg <me@rgrinberg.com> <!-- ps-id: d0bf377b-7729-4f6c-a42c-897e2b3b6f82 -->
416475a
to
8922377
Compare
ping @Leonidas-from-XIV |
I still think it might be good to provide the package name and version in the message and not just the location. |
How would that information help? It's irrelevant to the error itself, therefore distracting. More is not better when it comes to error messages. |
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.
Looks good. The only concern I have is that the filename to the offending file might not be a particularly useful one, especially if it is read from a checkout in a cache directory or from a git object.
But lets not get future concerns prevent current improvements.
Indeed, that's a good point. We could still be more helpful in such situations by including the commit, the repository URL, and the path. So at least the user knows where to find the file. We could even copy the file to a temporary directory. |
We don't have the exact locations, but we can at least point to the
relevant files.
Signed-off-by: Rudi Grinberg me@rgrinberg.com