Conversation
gpsamson
left a comment
gpsamson
left a comment
There was a problem hiding this comment.
Awesome start overall, especially on the Enhanced Ecommerce stuff! 😄I have left some minor comments around styling and gtag.js calls which don't exactly match the documentation made available by Google.
More importantly, I have highlighted places where this integration does not maintain parity with the Google Analytics integration that exists today. It is my understanding that it is not clear what pre-existing functionality and settings should be essentially copied over to this Gtag integration. With that in mind, I'm going to do a deeper dive into a parity comparison and follow up with more specific functionality and settings which should/should not be copied over.
cc @briemcnally
gpsamson
left a comment
gpsamson
left a comment
There was a problem hiding this comment.
Hey team, just looping back to let you know that there are two tickets remaining (DWEB-45, DWEB-46) before we feel that this can be merged in and released as beta. 😄
|
@gpsamson yes, will be working on them. |
Awesome, thank you so much! |
| gaSetting.site_speed_sample_rate = gaOptions.siteSpeedSampleRate; | ||
| } | ||
|
|
||
| if (gaOptions.useGoogleAmpClientId) { |
There was a problem hiding this comment.
Do you have a reference that use_amp_client_id is the right parameter? Can you provide a link to docs as well?
There was a problem hiding this comment.
@briemcnally I have referred setting from this reference: https://community.tealiumiq.com/t5/Client-Side-Tags/Google-Analytics-gtag-js-Tag-Setup-Guide/ta-p/30743
briemcnally
left a comment
briemcnally
left a comment
There was a problem hiding this comment.
Looks great! Just left a few minor comments @vjnathe-webonise but otherwise looks good. Will await @gpsamson to also do a final review 😄 👍
|
@briemcnally resolved the PR comments, let me know if any other input. |
gpsamson
left a comment
gpsamson
left a comment
There was a problem hiding this comment.
Nice work! The new changes look OK on my end.
|
@briemcnally @gpsamson Raised the PR against base repo: |
3 participants
What does this PR do?
GTAG destination
Are there breaking changes in this PR?
No.
Any background context you want to provide?
Customers will be able to use Segment to easily send
data to the Google ecosystem, using Google’s
preferred method—gtag.
Is there parity with the server-side/android/iOS integration components (if applicable)?
NA
Does this require a new integration setting? If so, please explain how the new setting works
Yes
Links to helpful docs and other external resources
https://developers.google.com/gtagjs