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

Use the exception log arg to log exception #6

Merged
merged 2 commits into from
Jun 7, 2021

Conversation

oxinabox
Copy link
Contributor

@oxinabox oxinabox commented Jun 7, 2021

The ConsoleLogger (and probably others) have special handling for log arguments called exception.
(Can also pass backtrace there but that is probably too spammy).

using @debug for this just seems annoying. If there is an error I want to know what it is.
Making me remember how to enable debug loading and then rerun is just annoying

src/citations.jl Outdated Show resolved Hide resolved
src/citations.jl Outdated
@@ -12,8 +12,7 @@ function get_citation(pkg)
try
import_bibtex(bib_path)
catch e
@warn("There was an error reading the CITATION.bib file for $(pkg.name)")
@debug e
@warn("There was an error reading the CITATION.bib file for $(pkg.name)" exception=e)
Copy link
Contributor Author

@oxinabox oxinabox Jun 7, 2021

Choose a reason for hiding this comment

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

Arguably this should bne

Suggested change
@warn("There was an error reading the CITATION.bib file for $(pkg.name)" exception=e)
@error "There was an error reading the CITATION.bib file for $(pkg.name)" exception=e

This was an error, we were not even able to recover from it correctly.
we kept running cos we know it is safe, but the final output is wrong (because it is incomplete)

@SebastianM-C
Copy link
Owner

Thank you, I didn't know about exception

@SebastianM-C SebastianM-C merged commit 1b27d58 into SebastianM-C:master Jun 7, 2021
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