-
Notifications
You must be signed in to change notification settings - Fork 0
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
Initial implementation of required functionality #1
Conversation
Co-authored-by: Denis Voytyuk <[email protected]> a
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.
There are some questions about usage and documenting less obvious behaviour.
Test for WithCause
I see another draft PR merged without changing the title and putting the description. Can we please improve the title and the desc here before we merge it? |
@e-sumin yes, can we get the final version in a single PR? |
internal/caller/frame.go
Outdated
|
||
// GetFrame returns information about a caller function at the specified depth | ||
// above this function in the the call stack. | ||
func GetFrame(depth int) Frame { |
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.
This function is only called in the test as I understand.
Is there some other use for it?
If not, should it be moved to the test or maybe test helper file?
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.
Thanks for pointing, in last commit I've extracted part which is common for marshalling code and for tests, and removed all irrelevant things.
Please re-review this commit.
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.
LGTM apart from internal/caller/frame.go
file.
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.
LGTM
Implementing functionality proposed in design proposal #2