Skip to content

Conversation

@tehut
Copy link

@tehut tehut commented Dec 30, 2019

Didn't expect this to be so large, I expect that a lot of the length is in the questions section on logger.go.

I tried including questions inline, if we hate it that's fine. Especially since this is on an open repo. I'll definitely delete that and any comments before asking for a merge.

Also, I didn't rebase my commits down to a single commit but can do that or squash on merge, I'm fine with either.

Error(string, ...interface{})
}

// Error wraps the go-hclog Error function and logs a separate error for each argument in the error object passed
Copy link
Author

Choose a reason for hiding this comment

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

I want to try keeping a "discarded attempts" section in my code reviews for now. I'll flag them all to be removed before merge but I thought we could try it. I'm hoping it helps me be able to keep track of multiple attempts. Again, may well discard.

@hashicorp-cla
Copy link

hashicorp-cla commented Sep 30, 2022

CLA assistant check

Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement

Learn more about why HashiCorp requires a CLA and what the CLA includes


1 out of 2 committers have signed the CLA.

  • tehut
  • Tehut Getahun

Tehut Getahun seems not to be a GitHub user.
You need a GitHub account to be able to sign the CLA.
If you have already a GitHub account, please add the email address used for this commit to your account.

Have you signed the CLA already but the status is still pending? Recheck it.

@mitar
Copy link

mitar commented Jan 7, 2024

I do not like this because it makes it very hard to use some other logger. Maybe it would be better to use new golang's structured logger? Is structured logging used at all? Otherwise standard logger is probably good enough?

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.

3 participants