Lotame add vendor #3021
Lotame add vendor #3021
Conversation
|
/to @avimehta |
| "vars": { | ||
| "account": 12345 | ||
| }, | ||
| "extraUrlParams": { |
There was a problem hiding this comment.
adding extraUrlParams is not needed here. That is a platform level thing and publishers are probably aware of them. Plus, you could add more meaningful examples to lotame product documentation.
There was a problem hiding this comment.
The extraUrlParams were added as an example for our clients. They were left intentionally generic to illustrate that they could be anything and that the specifics are dependent on case. Would you prefer I remove them?
There was a problem hiding this comment.
I think keeping it here is fine. But can you change the values to something meaningful? What do you expect publishers to use this for most?
|
please fix the lint errors, test that is failing. rest looks good to me. |
|
@avimehta - I made the changes for the trailing commas. Did I need to do anything else? |
|
yes, you'll need to make a change to vendors-requests.json to make the test pass. Once you do that and tests pass, please squash your commits and rebase. |
|
Ok, fixed the test, squashed the commits, it should be good to go. |
| @@ -20,7 +20,7 @@ | |||
| }, | |||
| "colanalytics": { | |||
| "host": "https://ase.clmbtech.com", | |||
| "base": "https://ase.clmbtech.com/message", | |||
| "base": "https://ase.clmbtech.com/message", | |||
There was a problem hiding this comment.
i think an extra space got added here.
…ytics usage reference with an entry for the lotame analytics service. resolving conflicts Added example for Lotame analytics. Updated the Lotame bcp url to use the proper new format Added trailing commas because that is the desired format. Added test to vendor-requests Updated the Lotame example to use more descriptive values Removed extraneous space
|
How about now? |
|
@dvoytenko Can you start the tests again? I think it is unrelated to the change in the PR. |
|
@avimehta started. |
|
@avimehta all good. feel free to merge. |
References: #3020