-
Notifications
You must be signed in to change notification settings - Fork 3
chore: add helper script to auto update analytics #68
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
chore: add helper script to auto update analytics #68
Conversation
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.
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Co-authored-by: Copilot <[email protected]>
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.
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
scripts/update-sr-plugin.js:7
- [nitpick] The variable 'pluginName' is declared but not used in the shown diff. Consider removing it if it is no longer required to reduce potential confusion.
const pluginName = 'sessionReplayPlugin';
Co-authored-by: Copilot <[email protected]>
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.
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
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.
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
Co-authored-by: Copilot <[email protected]>
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.
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (2)
scripts/update-sr-plugin.js:7
- The variable 'pluginName' is defined but not used anywhere in this script; consider removing it to clean up the code.
const pluginName = 'sessionReplayPlugin';
scripts/helpers.js:4
- Consider adding unit tests for the downloadAndDecompress helper to validate its behavior with various response scenarios, including error cases and proper decompression.
function downloadAndDecompress(url) {
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.
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
scripts/update-analytics-plugin.js:20
- Consider adding unit or integration tests for the update-analytics-plugin.js script to verify the version prompt, download, decompression process, and file update behavior.
(async () => {
Thanks @daniel-graham-amplitude for adding it. But I just realized that we shouldn't replace it with
To automate this, something we are missing
|
Nice! Glad to see this added, but I don't have full context what's required here for the browser SDK though. |
Co-authored-by: Xinyi Ye <[email protected]>
No problem Lew. I just wanted to include you because you had done the session replay version of this. |
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.
Thank you Dan! LGTM
Added a new Node script, "node ./scripts/update-analytics-plugin", that will 1. prompt the user for a version number; 2. download and compress the analytics script from the CDN, with that version number; 3. write the contents to the "amplitude-wrapper.js"
I tested this by following this guide and then ran it on a local "ngrok" server. I had built analytics browser with version 2.11.11 and can confirm that it worked and could send an event to Amplitude from
window.amplitudeGTM
(Also, this PR moved "downloadAndDecompress" to a re-usable helper function used by the two scripts)