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

improved error logging #5

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

wardviaene
Copy link
Contributor

It's nicer to have just the error and an exit 1, rather than a panic. Great utility otherwise! 👍

@orfin
Copy link
Contributor

orfin commented Dec 26, 2017

Thanks for contribution @wardviaene !

Just wondering:

  1. Why this change is needed? We are invoking Panic as this is an error that is unexpected and must stop the rest of application flow.
  2. Is it possible to omit if statement and just do log.println(err)?
  3. Why not log.Fatal?

@orfin
Copy link
Contributor

orfin commented Feb 14, 2018

@wardviaene ping :-)

@wardviaene
Copy link
Contributor Author

  1. How I understand, panic() should be used when there's something wrong in the the code/library, not when an external factor influences the run of an application. It's nicer to have the exact error returned with an exit code of 1.
  2. Indeed, it's only when you'd want to return different messages / codes for different scenarios
  3. log.Fatal seems to be a better choice indeed, also does os.Exit(1)

berga pushed a commit to berga/aws-env that referenced this pull request Sep 29, 2021
Added format option to not wrap param value in single quotes
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