Tappx Bid Adapter: minor updates and universal id added - #6504
Conversation
|
This pull request introduces 6 alerts when merging 8700881 into c5cd4e2 - view on LGTM.com new alerts:
|
smenzer
left a comment
smenzer
left a comment
There was a problem hiding this comment.
make sure you don't include the packgage-lock.json in your PR, please.
also, I would suggest adding a test to ensure that the payload.user gets set properly with values from the eids array.
| let user = {}; | ||
| user.ext = {}; | ||
| user.ext.eids = eidsArr; | ||
| payload.user = user; |
There was a problem hiding this comment.
this could be written in a single line like this:
| let user = {}; | |
| user.ext = {}; | |
| user.ext.eids = eidsArr; | |
| payload.user = user; | |
| payload.user = { | |
| ext: { | |
| eids: eidsArr | |
| } | |
| }; |
|
This pull request introduces 6 alerts when merging 88dfa06 into c5cd4e2 - view on LGTM.com new alerts:
|
|
This pull request introduces 6 alerts when merging 37d9f96 into c5cd4e2 - view on LGTM.com new alerts:
|
|
First of all, thank you for your comments @smenzer Thank you |
|
I would just untrack the package-lock file in this pr or you can update it to match the master branch. |
|
I've just updated for matching the package-lock.json from master. |
|
This pull request introduces 6 alerts when merging e5777a2 into c5cd4e2 - view on LGTM.com new alerts:
|
|
Comment says add uid, but code seems to pass all id's? Do you drop the rest server side? Can you clarify with a docs pr? |
|
@prebidtappx 👆 question and if you could please update docs before we merge 🙏 |
|
Hello @patmmccann @ChrisHuie |
4 participants
Type of change
Description of change
Other information