gen: Support go.label on struct fields #377
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
abhinav
requested review from
mh-park and
kriskowal
and removed request for
mh-park
|
CC @cl1337 |
This generalizes the labelling logic (go.label) to support any NamedEntity. To clarify: This isn't actually adding support for new entities, only generalizing the logic so that it can be re-used for struct field labels in the future. As part of generalization, this logic was moved to its own file.
This adds support for the `go.label` annotation on struct fields. Specifying this label on struct fields has two effects: - The name used for the field during logging with Zap will use the value specified in `go.label`. - The name of the field used while converting the struct to/from JSON will use the value specified in `go.label` **unless** a different value was specified with a `go.tag = 'json:".."'` annotation. I don't completely support making any assurances about the JSON representation of Thrift types but it seems to me that that ship sailed when we added the `json` tags. Resolves #370
kriskowal
approved these changes
Aug 27, 2018
abhinav
force-pushed
the
abg/go-label-fields
branch
from
faa2354 to
8687258
Compare
|
@abhinav Should we be using go.label for |
mh-park
approved these changes
Aug 27, 2018
|
Good question. No, because the I'll add this information to the commit message. |
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds support for the
go.labelannotation on struct fields.Specifying this label on struct fields has two effects:
value specified in
go.label.will use the value specified in
go.labelunless a differentvalue was specified with a
go.tag = 'json:".."'annotation. I don'tcompletely support making any assurances about the JSON
representation of Thrift types but it seems to me that that ship
sailed when we added the
jsontags.Note that this intentionally leaves out the
String()methods forstructs. Those methods currently output valid Go syntax with an
implication that the printed code can be used to rebuild the Go object.
Resolves #370
(This PR has another refactoring/cleanup commit that can be reviewed
separately.)