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

Merge upstream fix for array/slice support #3

Merged
merged 3 commits into from
Aug 14, 2024

Conversation

sgtsquiggs
Copy link
Collaborator

@sgtsquiggs sgtsquiggs commented Aug 12, 2024

Upstream fixed issue where zap-logfmt would panic when attempting to log an array/slice value

jsternberg and others added 3 commits August 23, 2022 12:07
This adds support for using `AddReflected` or `AppendReflected` with an
array or slice.

It also resolves an issue where using `AddReflected` would invoke
`Elem()` on the reflected value which was an illegal usage of this
function and would cause a panic. If it hadn't caused a panic, it would
have likely resulted in an infinite loop anyway.

This also refactors the code slightly so `AddReflected` and
`AppendReflected` no longer have duplicate code and `AppendReflected` is
the primary section of code.
…-panic

Add array/slice support for reflected values
@sgtsquiggs
Copy link
Collaborator Author

We'll need to create a new version tag once this is done - I don't think cicd will do it; @anishjayavant can you or someone in infra do this once this is merged?

@sgtsquiggs sgtsquiggs merged commit 1dfaba7 into master Aug 14, 2024
3 checks passed
@sgtsquiggs sgtsquiggs deleted the mcrenshaw/merge-upstream-improvements branch August 14, 2024 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants