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

avoid creating duplicate stack traces #1

Closed
wants to merge 1 commit into from

Conversation

gregwebs
Copy link

@gregwebs gregwebs commented Sep 5, 2018

This is done by using the new AddStack function instead of WithStack.
There is also a StackError utility function exposed.

@lysu
Copy link
Collaborator

lysu commented Sep 5, 2018

Hi @gregwebs , we need make a decisioin how to identity duplicate stack~?

there will be much AddStack call in TiDB before we remove middle used call(near every level), IMHO, bottom-up mark hasStack flag make higher cost than recurse call GetStackTracer(although pingcap/tidb@3712fc1 make more modification)~ 😆 ?

@lysu
Copy link
Collaborator

lysu commented Sep 5, 2018

expect that, we meet other problem, tidb has too many pending PR and we prefer rename some method name of pkg/errors to decrease old code modification.

e.g.

AddStack ==> Trace
Wrap ==> Annotate
Wrapf ==> Annotatef

can we do them in this repo, how do you think abouth it~? 🐱

This is done by using the new AddStack function instead of WithStack.
There is also a StackError utility function exposed.
@gregwebs
Copy link
Author

gregwebs commented Sep 5, 2018

Sounds good. Addressed in this PR: https://github.com/lysu/errors/pull/2

@gregwebs gregwebs closed this Sep 5, 2018
@lysu lysu mentioned this pull request Sep 6, 2018
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