Skip to content

Conversation

@ZetaoZhuang
Copy link
Contributor

@ZetaoZhuang ZetaoZhuang commented Aug 16, 2022

Reason for Change:

extend fieldStringer to support zap ErrorType and BoolType, so zap.Error and zap.Bool fields can be better map to strings while constructing TraceTelemetry

Issue Fixed:

Requirements:

Notes:

@ZetaoZhuang ZetaoZhuang requested a review from rbtr as a code owner August 16, 2022 22:13
@rbtr rbtr force-pushed the extend_fieldStringer branch from e3798e7 to 86cd44b Compare August 16, 2022 23:07
rbtr
rbtr previously approved these changes Aug 16, 2022
@ZetaoZhuang ZetaoZhuang enabled auto-merge (squash) August 17, 2022 19:55
@ZetaoZhuang ZetaoZhuang force-pushed the extend_fieldStringer branch from 86cd44b to 30be0bd Compare August 17, 2022 21:27
@ZetaoZhuang ZetaoZhuang force-pushed the extend_fieldStringer branch 2 times, most recently from 3587e90 to 371067e Compare August 18, 2022 21:47
@ZetaoZhuang ZetaoZhuang requested a review from rbtr August 18, 2022 22:22
zapai/encoder.go Outdated

case zapcore.BoolType:
return strconv.FormatBool(f.Integer == 1)

Copy link
Collaborator

Choose a reason for hiding this comment

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

why do you have all this extra whitespace? 🥲

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you mean the empty line right? removed.

@ZetaoZhuang ZetaoZhuang force-pushed the extend_fieldStringer branch from 371067e to b85e617 Compare August 18, 2022 23:52
@ZetaoZhuang ZetaoZhuang force-pushed the extend_fieldStringer branch from b85e617 to 917c503 Compare August 18, 2022 23:53
@ZetaoZhuang ZetaoZhuang requested a review from rbtr August 18, 2022 23:54
@rbtr rbtr disabled auto-merge August 19, 2022 18:10
@rbtr rbtr merged commit 62b825c into master Aug 19, 2022
@rbtr rbtr deleted the extend_fieldStringer branch August 19, 2022 18:10
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