refactor(openrtb): Extension refactor part1 - BidRequest Device Impression #115
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
added 4 commits
February 8, 2022 14:40
… differences between output and expected data.
… differences between output and expected data.
…m to json.RawMessage.
Here are two reason to do that:
1: Avoid the loss of precise while unmarshal data to interface{}. See https://vungle.atlassian.net/browse/PBJ-3092
2: Avoid the change between interface{} and specified structs. Using json.RawMessage can make sure there is only one convert from the beginning.
|
Files changed in PR, revertig code review status. |
added 3 commits
February 8, 2022 15:08
…keep unchanged. If we update one struct and use "make generate" to get the easyjson file, all file will be removed and regenerate. Here will be code redundant. E.g. while enerating Device, the framework will find its sub struct Geo, but Geo is after Device in alphabet order, so the easyjson framework will generate the code for Geo in device_easyjson.go. However, if we use the generate tools of Goland, that is the green triangle in the front of device.go file, the easyjson framework will use Geo.Marshal method there. In order to keep the code unchanged and make an uniform way to generate code, we use "make generate" command here.
|
Files changed in PR, revertig code review status. |
…e/vungo into wip-pbj-3419-raw-message-extension
HaibinZhang-Vungle
changed the title
openrtb/openrtbutil/request_test.go
Outdated
| "localhost/#宝%贝%约%么%", | ||
| reflect.TypeOf((*url.Error)(nil)), | ||
| }, | ||
| //{ |
There was a problem hiding this comment.
Sure. Remove unused lines.
yingchen0706v
previously approved these changes
Apr 19, 2022
HaibinZhang-Vungle
changed the title
joseph-shi-vungle
previously approved these changes
Apr 21, 2022
…ice-ext refactor(openrtb): Use json.RawMessage as the extension object part3 - Device
HaibinZhang-Vungle
dismissed stale reviews from joseph-shi-vungle and yingchen0706v
via
f1a808a
|
Files changed in PR, revertig code review status. |
Remove unused lines.
…ression-ext refactor(openrtb): Use json.RawMessage as the extension object part4- Impression
HaibinZhang-Vungle
changed the title
HaibinZhang-Vungle
changed the title
HaibinZhang-Vungle
changed the title
joseph-shi-vungle
approved these changes
May 9, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in 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.
Update extension fields of BidRequest and its sub structs. Change them to json.RawMessage.
Here are two reason to do that:
1: Avoid the loss of precise while unmarshal data to interface{}. The build-in json and easyjson both use the type of float64 to parse a number in json string. If it is an int64 number, some information will loss.
2: Avoid the change between interface{} and specified structs. Using json.RawMessage can make sure there is only one convert from the beginning.