-
Notifications
You must be signed in to change notification settings - Fork 3
CER-893 Feature/eloqua plugin to sd kv5 #21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
src/eloqua/eloqua.js
Outdated
| paths: { | ||
| elq: "//img.en25.com/i/elqCfg.min", | ||
| CerosSDK: "//sdk.ceros.com/standalone-player-sdk-v3" | ||
| CerosSDK: '//sdk.ceros.com/standalone-player-sdk-v5' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically single or double quotes don't make a difference in js, but I like to keep consistency with the rest of the file, so if the file is already using double quotes I would use that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated the single quotes to double quotes, committed and pushed the change. Please let me know what my next steps should be.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hi looks good, thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I click on 'Merge pull request'? It is my first time going through this procedure, so I'm a little unsure what I should do next to complete the change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After code 👍 you'll need a functional 👍 from QA or someone who tests the story before merging
rolfisub
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi there just a small change
|
Code 👍 |
Updated the CerosSDK link from //sdk.ceros.com/standalone-player-sdk-v3 to //sdk.ceros.com/standalone-player-sdk-v5.
Updated experience.subscribe to experience.on and PAGE_CHANGE to PAGE_CHANGED to be in sync with sdk-v5.