docs: add contribution guide for device mode integration #1979
Conversation
📝 WalkthroughWalkthroughThe pull request introduces a new section titled "Guide to develop your first RudderStack integration" in the Changes
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant Guide as CONTRIBUTING.md
participant Env as Development Environment
participant Integration as Device Mode Integration
Dev->>Guide: Read integration development guide
Dev->>Env: Set up development environment
Dev->>Integration: Define core components
Dev->>Integration: Implement integration code
Dev->>Integration: Configure testing setup
Dev->>Integration: Build and test integration
Dev->>Integration: Prepare documentation
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🧹 Nitpick comments (3)
CONTRIBUTING.md (3)
54-64: LGTM! Consider adding a brief comparison of integration modes.The introduction effectively sets the context and links to essential documentation. To enhance clarity, consider adding a brief comparison of when to use Cloud Mode vs Device Mode integration.
Add a brief comparison like:
2. **Device Mode Integration**: Events are sent directly from the client to the destination in this mode + +Choose Device Mode when: +- Real-time user interaction is required +- Client-side features like user tracking or A/B testing are needed +- Reduced server load is desired🧰 Tools
🪛 LanguageTool
[uncategorized] ~55-~55: This verb may not be in the correct form. Consider using a different form for this context.
Context: ...ty/) channel.
---- # Guide to develop your first device mode RudderStack inte...(AI_EN_LECTOR_REPLACEMENT_VERB_FORM)
127-136: Enhance testing setup documentation.The testing setup section would benefit from:
- Examples of environment variable values
- Troubleshooting common issues
- Validation steps to confirm successful setup
Consider adding:
2. Configure test environment: - Modify `public/index.html` for mock source config - Set environment variables (WRITE_KEY, DATAPLANE_URL) + Example: + ```bash + export WRITE_KEY=<your_write_key> + export DATAPLANE_URL=http://localhost:8080 + ``` + + Validation: + 1. Check the network tab for successful SDK initialization + 2. Verify events are being sent to your destination + + Common issues: + - If events aren't appearing, check browser console for errors + - Ensure dataplane URL is accessible
164-171: Expand best practices with security and performance guidelines.Consider adding specific guidelines for:
- Security best practices (data handling, PII considerations)
- Performance optimization
- Error handling standards
Add these points:
- Keep code modular and maintainable +- Follow security best practices: + - Never log sensitive data or PII + - Use secure protocols for data transmission + - Implement rate limiting where appropriate +- Optimize performance: + - Minimize SDK size and dependencies + - Implement proper error handling and retries + - Cache expensive operations when possible
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CONTRIBUTING.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
CONTRIBUTING.md
[uncategorized] ~55-~55: This verb may not be in the correct form. Consider using a different form for this context.
Context: ...ty/) channel.
---- # Guide to develop your first device mode RudderStack inte...
(AI_EN_LECTOR_REPLACEMENT_VERB_FORM)
[uncategorized] ~148-~148: The preposition ‘to’ seems more likely in this position.
Context: ...l configuration** - Add config files in src/configurations/destinations - ...
(AI_HYDRA_LEO_REPLACE_IN_TO)
🔇 Additional comments (1)
CONTRIBUTING.md (1)
173-180: LGTM! Comprehensive reference section.
The references section provides valuable resources including community links, workshop recordings, and documentation.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1979 +/- ##
========================================
Coverage 61.28% 61.28%
========================================
Files 484 484
Lines 16612 16612
Branches 3327 3352 +25
========================================
Hits 10181 10181
+ Misses 5224 5166 -58
- Partials 1207 1265 +58 ☔ View full report in Codecov by Sentry. |
size-limit report 📦
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
CONTRIBUTING.md (3)
16-16: Fix grammar: Add missing articleAdd "the" before "list" for proper grammar.
-we can add you to list of approved contributors +we can add you to the list of approved contributors🧰 Tools
🪛 LanguageTool
[uncategorized] ~16-~16: Possible missing article found.
Context: ...to the [CLA][CLA] we can add you to list of approved contributors and review the...(AI_HYDRA_LEO_MISSING_THE)
34-34: Fix subject-verb agreementCorrect the verb form to match the plural subject "folders".
-folders that contains the integration SDKs +folders that contain the integration SDKs🧰 Tools
🪛 LanguageTool
[uncategorized] ~34-~34: Loose punctuation mark.
Context: ...ntents. -npm run build:integrations: This outputs dist/cdn/legacy and **...(UNLIKELY_OPENING_PUNCTUATION)
[grammar] ~34-~34: Possible subject-verb agreement error detected.
Context: ...** and dist/cdn/modern folders that contains the integration SDKs. > We use **rollu...(PLURAL_THAT_AGREEMENT)
227-233: Expand the best practices sectionConsider adding these important best practices:
- Error handling and logging strategies
- Performance optimization guidelines
- Testing strategies (unit, integration, e2e)
- Version compatibility considerations
- Security best practices
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package.jsonis excluded by!**/*.json
📒 Files selected for processing (1)
CONTRIBUTING.md(3 hunks)
🧰 Additional context used
🪛 LanguageTool
CONTRIBUTING.md
[uncategorized] ~16-~16: Possible missing article found.
Context: ...to the [CLA][CLA] we can add you to list of approved contributors and review the...
(AI_HYDRA_LEO_MISSING_THE)
[uncategorized] ~32-~32: Loose punctuation mark.
Context: ...are: - npm run build:browser:modern: This outputs dist/cdn/modern folder...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~33-~33: Loose punctuation mark.
Context: ...contents. - npm run build:npm:modern: This outputs dist/npm/modern folder...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~34-~34: Loose punctuation mark.
Context: ...ntents. - npm run build:integrations: This outputs dist/cdn/legacy and **...
(UNLIKELY_OPENING_PUNCTUATION)
[grammar] ~34-~34: Possible subject-verb agreement error detected.
Context: ...** and dist/cdn/modern folders that contains the integration SDKs. > We use **rollu...
(PLURAL_THAT_AGREEMENT)
[style] ~42-~42: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ... are squashed when merged, but rebasing prior to merge gives you better control over the...
(EN_WORDINESS_PREMIUM_PRIOR_TO)
🪛 Markdownlint (0.37.0)
CONTRIBUTING.md
136-136: Expected: 2; Actual: 3
Unordered list indentation
(MD007, ul-indent)
138-138: Expected: 2; Actual: 3
Unordered list indentation
(MD007, ul-indent)
🔇 Additional comments (2)
CONTRIBUTING.md (2)
113-133: Add documentation and error handling to the integration class example
This is a critical example that needs more robust documentation and error handling.
53-243: Well-structured and comprehensive guide
The device mode integration guide is well-organized and provides clear, actionable steps for contributors. It effectively covers the entire development lifecycle from setup to deployment.
🧰 Tools
🪛 Markdownlint (0.37.0)
136-136: Expected: 2; Actual: 3
Unordered list indentation
(MD007, ul-indent)
138-138: Expected: 2; Actual: 3
Unordered list indentation
(MD007, ul-indent)
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
CONTRIBUTING.md (2)
85-85: Fix typo in the word "integration".-If _device mode_ integration does not seem suitable, go ahead with the _cloud mode_ inregration development instead +If _device mode_ integration does not seem suitable, go ahead with the _cloud mode_ integration development instead🧰 Tools
🪛 LanguageTool
[style] ~85-~85: To elevate your writing, consider using a more formal and concise alternative here.
Context: ...de_ integration does not seem suitable, go ahead with the cloud mode inregration devel...(GO_AHEAD_PROCEED)
74-84: Enhance the device mode trade-offs section.Consider adding performance impact metrics or examples to make the trade-offs more concrete. For example:
- Bundle size impact
- Typical load time differences
- Common ad blockers that might affect the integration
🧰 Tools
🪛 LanguageTool
[style] ~82-~82: To elevate your writing, try using a synonym here.
Context: ...y by adding third-party SDKs * Makes it hard to collect first-party data * Prone...(HARD_TO)
[misspelling] ~83-~83: Did you mean the verb “add” instead of the noun ‘ad’ (advertisement)?
Context: ...collect first-party data * Prone to ad blockers If device mode integration ...(AD_ADD)
🪛 Markdownlint (0.37.0)
76-76: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
77-77: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
81-81: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
82-82: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
83-83: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CONTRIBUTING.md(3 hunks)
🧰 Additional context used
🪛 LanguageTool
CONTRIBUTING.md
[uncategorized] ~33-~33: Loose punctuation mark.
Context: ...are: - npm run build:browser:modern: This outputs dist/cdn/modern folder...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~34-~34: Loose punctuation mark.
Context: ...contents. - npm run build:npm:modern: This outputs dist/npm/modern folder...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~35-~35: Loose punctuation mark.
Context: ...ntents. - npm run build:integrations: This outputs dist/cdn/legacy and **...
(UNLIKELY_OPENING_PUNCTUATION)
[grammar] ~35-~35: Possible subject-verb agreement error detected.
Context: ...** and dist/cdn/modern folders that contains the integration SDKs. > We use **rollu...
(PLURAL_THAT_AGREEMENT)
[style] ~43-~43: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ... are squashed when merged, but rebasing prior to merge gives you better control over the...
(EN_WORDINESS_PREMIUM_PRIOR_TO)
[style] ~82-~82: To elevate your writing, try using a synonym here.
Context: ...y by adding third-party SDKs * Makes it hard to collect first-party data * Prone...
(HARD_TO)
[misspelling] ~83-~83: Did you mean the verb “add” instead of the noun ‘ad’ (advertisement)?
Context: ...collect first-party data * Prone to ad blockers If device mode integration ...
(AD_ADD)
[style] ~85-~85: To elevate your writing, consider using a more formal and concise alternative here.
Context: ...de_ integration does not seem suitable, go ahead with the cloud mode inregration devel...
(GO_AHEAD_PROCEED)
[uncategorized] ~104-~104: Loose punctuation mark.
Context: ... packages directory. * analytics-js: The main JavaScript SDK * `analytics-js...
(UNLIKELY_OPENING_PUNCTUATION)
[style] ~244-~244: Consider a shorter alternative to avoid wordiness.
Context: ... ### Configurations for the integration In order to allow user to configure your integratio...
(IN_ORDER_TO_PREMIUM)
🪛 Markdownlint (0.37.0)
CONTRIBUTING.md
76-76: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
77-77: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
81-81: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
82-82: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
83-83: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
104-104: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
105-105: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
106-106: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
107-107: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
111-111: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
112-112: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
113-113: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
114-114: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
115-115: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
116-116: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
125-125: Expected: 0; Actual: 3
Unordered list indentation
(MD007, ul-indent)
126-126: Expected: 0; Actual: 3
Unordered list indentation
(MD007, ul-indent)
127-127: Expected: 0; Actual: 3
Unordered list indentation
(MD007, ul-indent)
161-161: Expected: 2; Actual: 3
Unordered list indentation
(MD007, ul-indent)
163-163: Expected: 2; Actual: 3
Unordered list indentation
(MD007, ul-indent)
🔇 Additional comments (4)
CONTRIBUTING.md (4)
13-13: LGTM: Table of contents update is accurate.
The new section is properly added to the table of contents with correct formatting and link.
138-158: Add documentation and error handling to the integration class example.
This comment was previously made and remains valid. The integration class example needs parameter documentation, error handling, and return type information.
192-232: Enhance security in test configuration example.
This security concern was previously raised and remains valid. The test configuration example should include notes about using environment variables for sensitive values.
281-288: LGTM: References section is comprehensive.
The references section provides valuable resources for contributors, including community links, workshop recordings, and documentation.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
CONTRIBUTING.md (3)
13-13: LGTM! Minor markdown formatting suggestions.The new section is properly added to the TOC and matches the section title. Consider standardizing the unordered list style to use dashes (-) instead of asterisks (*) throughout the document for consistency.
Also applies to: 56-56
72-72: Fix typos and improve clarity.
- Line 72: Add "the" before "Following information"
- Line 85: Fix typo "inregration" to "integration"
-Should you choose device mode integration over cloud mode integration? Following information should help you make the decision. +Should you choose device mode integration over cloud mode integration? The following information should help you make the decision. -If _device mode_ integration does not seem suitable, go ahead with the _cloud mode_ inregration development instead +If _device mode_ integration does not seem suitable, proceed with the _cloud mode_ integration development insteadAlso applies to: 85-85
🧰 Tools
🪛 LanguageTool
[uncategorized] ~72-~72: Possible missing article found.
Context: ...ntegration over cloud mode integration? Following information should help you make the de...(AI_HYDRA_LEO_MISSING_THE)
291-297: Enhance best practices with examples.Consider adding concrete examples of good vs bad practices for each guideline to help developers better understand the recommendations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CONTRIBUTING.md(3 hunks)
🧰 Additional context used
🪛 LanguageTool
CONTRIBUTING.md
[uncategorized] ~33-~33: Loose punctuation mark.
Context: ...are: - npm run build:browser:modern: This outputs dist/cdn/modern folder...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~34-~34: Loose punctuation mark.
Context: ...contents. - npm run build:npm:modern: This outputs dist/npm/modern folder...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~35-~35: Loose punctuation mark.
Context: ...ntents. - npm run build:integrations: This outputs dist/cdn/legacy and **...
(UNLIKELY_OPENING_PUNCTUATION)
[grammar] ~35-~35: Possible subject-verb agreement error detected.
Context: ...** and dist/cdn/modern folders that contains the integration SDKs. > We use **rollu...
(PLURAL_THAT_AGREEMENT)
[style] ~43-~43: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ... are squashed when merged, but rebasing prior to merge gives you better control over the...
(EN_WORDINESS_PREMIUM_PRIOR_TO)
[uncategorized] ~72-~72: Possible missing article found.
Context: ...ntegration over cloud mode integration? Following information should help you make the de...
(AI_HYDRA_LEO_MISSING_THE)
[style] ~82-~82: To elevate your writing, try using a synonym here.
Context: ...y by adding third-party SDKs * Makes it hard to collect first-party data * Prone...
(HARD_TO)
[misspelling] ~83-~83: Did you mean the verb “add” instead of the noun ‘ad’ (advertisement)?
Context: ...collect first-party data * Prone to ad blockers If device mode integration ...
(AD_ADD)
[style] ~85-~85: To elevate your writing, consider using a more formal and concise alternative here.
Context: ...de_ integration does not seem suitable, go ahead with the cloud mode inregration devel...
(GO_AHEAD_PROCEED)
[uncategorized] ~104-~104: Loose punctuation mark.
Context: ... packages directory. * analytics-js: The main JavaScript SDK * `analytics-js...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~170-~170: Possible missing article found.
Context: ...d, getUserId, etc. - rudderElement` object is a wrapper around the actual standard...
(AI_HYDRA_LEO_MISSING_THE)
[style] ~252-~252: Consider a shorter alternative to avoid wordiness.
Context: ...udder-integrations-config` repository) In order to allow user to configure your integratio...
(IN_ORDER_TO_PREMIUM)
[uncategorized] ~253-~253: Possible missing article found.
Context: ...-config` repository) In order to allow user to configure your integration via the R...
(AI_HYDRA_LEO_MISSING_A)
[grammar] ~268-~268: It seems that “to” is missing before the verb.
Context: ...immutable and should be intuitive * Include the integration's name, display name, s...
(MISSING_TO_BETWEEN_BE_AND_VB)
[uncategorized] ~274-~274: Possible missing preposition found.
Context: ...actices of configuration management:** Decide which configurations should be set via ...
(AI_HYDRA_LEO_MISSING_TO)
🪛 Markdownlint (0.37.0)
CONTRIBUTING.md
76-76: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
77-77: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
81-81: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
82-82: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
83-83: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
104-104: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
105-105: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
106-106: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
107-107: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
111-111: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
112-112: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
113-113: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
114-114: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
115-115: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
116-116: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
266-266: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
267-267: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
268-268: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
269-269: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
270-270: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
271-271: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
277-277: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
278-278: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
279-279: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
280-280: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
129-129: Expected: 0; Actual: 3
Unordered list indentation
(MD007, ul-indent)
130-130: Expected: 0; Actual: 3
Unordered list indentation
(MD007, ul-indent)
131-131: Expected: 0; Actual: 3
Unordered list indentation
(MD007, ul-indent)
167-167: Expected: 2; Actual: 3
Unordered list indentation
(MD007, ul-indent)
169-169: Expected: 2; Actual: 3
Unordered list indentation
(MD007, ul-indent)
197-197: Expected: 0; Actual: 3
Unordered list indentation
(MD007, ul-indent)
240-240: Expected: 0; Actual: 3
Unordered list indentation
(MD007, ul-indent)
241-241: Expected: 0; Actual: 3
Unordered list indentation
(MD007, ul-indent)
268-268: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
269-269: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
277-277: Expected: 0; Actual: 2
Unordered list indentation
(MD007, ul-indent)
278-278: Expected: 0; Actual: 2
Unordered list indentation
(MD007, ul-indent)
279-279: Expected: 0; Actual: 2
Unordered list indentation
(MD007, ul-indent)
280-280: Expected: 0; Actual: 2
Unordered list indentation
(MD007, ul-indent)
124-124: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
139-139: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (2)
CONTRIBUTING.md (2)
144-164: Add documentation and error handling to the integration class example.
The integration class example would benefit from parameter documentation and error handling as suggested in the previous review.
300-307: LGTM! Comprehensive references provided.
The references section includes valuable resources including documentation, community channels, and workshop recordings.
|
PR Description
Add a comprehensive step-by-step guide to develop device mode integration as a new section in
Contributing.mdLinear task (optional)
N/A
Cross Browser Tests
N/A
Sanity Suite
Security
Summary by CodeRabbit
CONTRIBUTING.mdfile.