chore(release): pulling release/3.5.0 into main #1681
Conversation
* fix: updated isLoaded and isReady conditions for mixpanel * chare: addressed review comments * fix: updated isLoaded and isReady conditions for mixpanel * fix: added extra tests * fix: updated tests
devops-github-rudderstack
requested review from
ItsSudip and
shrouti1507
as code owners
devops-github-rudderstack
requested review from
sandeepdsvs,
koladilip,
mihir-4116 and
a team
as code owners
WalkthroughThe recent updates focus on enhancing clarity and functionality across Ninetailed integration sample apps and the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1681 +/- ##
==========================================
+ Coverage 53.90% 53.93% +0.02%
==========================================
Files 461 461
Lines 15611 15616 +5
Branches 3091 3097 +6
==========================================
+ Hits 8415 8422 +7
+ Misses 5907 5878 -29
- Partials 1289 1316 +27 ☔ View full report in Codecov by Sentry. |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review Status
Configuration used: CodeRabbit UI
Files ignored due to path filters (4)
package-lock.jsonis excluded by!**/package-lock.json,!**/*.jsonpackage.jsonis excluded by!**/*.jsonpackages/analytics-js-integrations/package.jsonis excluded by!**/*.jsonpackages/analytics-js-integrations/project.jsonis excluded by!**/*.json
Files selected for processing (10)
- examples/integrations/Ninetailed/sample-apps/README.md (1 hunks)
- examples/integrations/Ninetailed/sample-apps/app-using-v3-cdn/README.md (2 hunks)
- examples/integrations/Ninetailed/sample-apps/app-using-v3-cdn/src/config.js (1 hunks)
- packages/analytics-js-integrations/CHANGELOG.md (1 hunks)
- packages/analytics-js-integrations/CHANGELOG_LATEST.md (1 hunks)
- packages/analytics-js-integrations/tests/integrations/Mixpanel/browser.test.js (5 hunks)
- packages/analytics-js-integrations/tests/integrations/Ninetailed/browser.test.js (2 hunks)
- packages/analytics-js-integrations/src/integrations/Mixpanel/browser.js (2 hunks)
- packages/analytics-js-integrations/src/integrations/Ninetailed/browser.js (2 hunks)
- sonar-project.properties (1 hunks)
Files skipped from review due to trivial changes (1)
- sonar-project.properties
Additional comments not posted (17)
examples/integrations/Ninetailed/sample-apps/app-using-v3-cdn/src/config.js (2)
2-2: The update toWRITE_KEYwith a placeholder value is appropriate for a sample app configuration.
3-8: The formatting adjustments to the exports improve readability and maintainability.packages/analytics-js-integrations/CHANGELOG_LATEST.md (3)
1-1: The version update to 3.0.3 follows semantic versioning practices, indicating backward-compatible bug fixes.
6-6: The bug fix entry for Ninetailed is clear and includes relevant links. The spelling of 'ninetailed' is correct in this context.
7-7: The bug fix entry for Mixpanel is clear and includes relevant links. The spelling concerns flagged by the static analysis are false positives.packages/analytics-js-integrations/src/integrations/Ninetailed/browser.js (2)
17-17: The introduction ofsendPageEventsbased onconfig.sendPageInDeviceis a logical addition, allowing for flexible configuration of page event tracking.
55-63: The conditional logic usingsendPageEventsin thepagemethod is correctly implemented, ensuring that page events are sent only whensendPageEventsis true.examples/integrations/Ninetailed/sample-apps/app-using-v3-cdn/README.md (1)
27-27: The semantic adjustments, including adding commas for clarity, enhance the readability of the documentation.packages/analytics-js-integrations/__tests__/integrations/Ninetailed/browser.test.js (2)
49-49: The inclusion of thesendPageInDeviceconfiguration parameter in the instantiation ofNinetailedfor test cases is a necessary update to accurately test the new behavior.
82-90: The updated test cases effectively reflect the new behavior controlled by thesendPageInDeviceparameter, ensuring comprehensive coverage.packages/analytics-js-integrations/__tests__/integrations/Mixpanel/browser.test.js (2)
18-26: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [21-33]
The addition of
loaded: expect.any(Function)to various test cases accurately reflects the Mixpanel integration's initialization process and ensures the tests simulate the SDK's loading behavior correctly.
81-117: The new tests for 'isLoaded' and 'isReady' functionalities are well-structured and necessary for verifying the updated behavior of the Mixpanel integration, ensuring comprehensive test coverage.packages/analytics-js-integrations/src/integrations/Mixpanel/browser.js (4)
69-69: The addition ofisNativeSDKLoadedis a clear way to track the loading state of the Mixpanel SDK, essential for accurately determining the SDK's readiness.
97-101: The modifications to theinitmethod, including settingisNativeSDKLoadedtotruewhen the SDK is loaded, accurately reflect the SDK's loading state.
105-105: The update to theisLoadedmethod to checkisNativeSDKLoadedis a more reliable way to determine if the SDK is loaded.
109-109: The update to theisReadymethod to callisLoadedfor determining readiness ensures consistency in how the integration checks for readiness.packages/analytics-js-integrations/CHANGELOG.md (1)
5-11: The changelog entry for version 3.0.3 clearly outlines the bug fixes for Ninetailed and Mixpanel integrations. Please ensure that the implementation and tests for these changes are thoroughly verified.
| 1. NPM package: This is for Production Use Case where rudderstack would be installed using npm install command and the app will initialise its instance. | ||
|
|
||
| 3. V3 CDN: This app can be used for any environment either for dev, staging or prod by providing the right configurations in config.js file. This uses the V3 SDK. | ||
| 2. V1.1 CDN: This app can be used for any environment either for dev, staging or prod by providing the right configurations in config.js file .This uses the legacy SDK. | ||
|
|
||
| For Customers: NPM one is advised and other ones are just testing purposes mainly for developers No newline at end of file | ||
| 3. V3 CDN: This app can be used for any environment either for dev, staging or prod by providing the right configurations in config.js file . This uses the V3 SDK. | ||
|
|
||
| For Customers: The NPM package is recommended for production use. The other versions are intended primarily for testing and development purposes. |
There was a problem hiding this comment.
The updated descriptions for the sample apps are clear and helpful. Minor grammatical improvements could be made for better readability. For example, consider adding a space after periods and using "in" instead of "for" when referring to environments.
- ...for any environment either for dev, staging or prod by providing the right configurations in config.js file .This uses...
+ ...in any environment, either for dev, staging, or prod, by providing the right configurations in the config.js file. This uses...Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| 1. NPM package: This is for Production Use Case where rudderstack would be installed using npm install command and the app will initialise its instance. | |
| 3. V3 CDN: This app can be used for any environment either for dev, staging or prod by providing the right configurations in config.js file. This uses the V3 SDK. | |
| 2. V1.1 CDN: This app can be used for any environment either for dev, staging or prod by providing the right configurations in config.js file .This uses the legacy SDK. | |
| For Customers: NPM one is advised and other ones are just testing purposes mainly for developers | |
| \ No newline at end of file | |
| 3. V3 CDN: This app can be used for any environment either for dev, staging or prod by providing the right configurations in config.js file . This uses the V3 SDK. | |
| For Customers: The NPM package is recommended for production use. The other versions are intended primarily for testing and development purposes. | |
| 1. NPM package: This is for Production Use Case where rudderstack would be installed using npm install command and the app will initialise its instance. | |
| 2. V1.1 CDN: This app can be used in any environment, either for dev, staging, or prod, by providing the right configurations in the config.js file. This uses the legacy SDK. | |
| 3. V3 CDN: This app can be used in any environment, either for dev, staging, or prod, by providing the right configurations in the config.js file. This uses the V3 SDK. | |
| For Customers: The NPM package is recommended for production use. The other versions are intended primarily for testing and development purposes. |
size-limit report 📦
|
|
@MoumitaM, can you link the release ticket in the description? |
Done |
👑 An automated PR
Summary by CodeRabbit
sendPageInDevicein the Ninetailed integration to control page event sending behavior.@rudderstack/analytics-js-integrationsrelated to Ninetailed page support and Mixpanel's readiness checks.sonar.projectVersionto reflect the new version increment.Release ticket:
https://linear.app/rudderstack/issue/SDK-1526/release-sdk-js-3rd-apr