-
Notifications
You must be signed in to change notification settings - Fork 26
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
Provide guidance links in mix tasks' Logger.error
output
#26
base: master
Are you sure you want to change the base?
Provide guidance links in mix tasks' Logger.error
output
#26
Conversation
Logger.error
output
* https://github.com/Artur-Sulej/excellent_migrations#checks | ||
* https://github.com/Artur-Sulej/excellent_migrations#assuring-safety |
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.
The link fragments here are inherently brittle, but if the README headings change without the appropriate fragment updates here, the user will still end-up on the README document, which is still a net UX win.
Thanks for the PR and using the lib! I was thinking about introducing something similar myself. My idea was to add README links to specific dangers that were detected. Additionally these links would point to README from a lib version that is used. For example After adjusting links to match check names, links could be built with this logic: def build_link(current_version, anchor) do
anchor =
check
|> Atom.to_astring()
|> String.replace("_", "-")
"#{@base_url}/blob/v#{current_version}/README.md##{anchor}"
end Anyway, that could be done in the future. |
I actually started down a path of trying to build out links and aborted due to not knowing if you'd be interested in a bigger change, but I hadn't considered tying the links to the particular release tag to avoid brittleness. That's 💯 Happy to take on the subsequent change once this first one goes in. |
@Artur-Sulej I'm curious how you're feeling about this. I just got around to auditing my dependencies on a side project, and realized my fork was behind origin. These warnings have aided me personally :) |
We just added
excellent_migrations
to a large repository with many newish Elixir developers contributing to it. I noticed that the currentLogger.error
output can be somewhat cryptic, and it isn't clear what the next steps are or where the errors originate.This pull request adds links to the README's 'Checks' and 'Assuring Safety' section to the error output of the mix tasks.
I haven't yet determined how to do the same for the credo check, but I'll happily add it if this first PR is welcomed.
Before
After