Update contributing guidelines #16926
Conversation
|
Warning Rate limit exceeded@michelle0927 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 10 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThe documentation for contributing components was updated. Requirements now include linking to relevant documentation in action descriptions, incrementing versions when affected files change, using Changes
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
![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)
docs-v2/pages/components/contributing/guidelines.mdx (3)
412-412: Style: Add missing period in "etc."
Abbreviations like "etc." require a period. Please changeetc)toetc.).🧰 Tools
🪛 LanguageTool
[style] ~412-~412: In American English, abbreviations like “etc.” require a period.
Context: ...et, encapsulate the API URL and token, etc). - Thecommon.mjs` module contains lo...(ETC_PERIOD)
415-415: Style: Add missing period in "etc."
Please updateetctoetc.to conform with American English abbreviation standards.🧰 Tools
🪛 LanguageTool
[style] ~415-~415: In American English, abbreviations like “etc.” require a period.
Context: ... asversion,dedupe,key,name, etc (those are specific to each component...(ETC_PERIOD)
422-422: Style: Add missing period in "etc."
The list ends withetc)—add the period:etc.).🧰 Tools
🪛 LanguageTool
[style] ~422-~422: In American English, abbreviations like “etc.” require a period.
Context: ...s (e.g.name,description,key, etc) and potentially redefining any inherit...(ETC_PERIOD)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
docs-v2/pages/components/contributing/guidelines.mdx(8 hunks)
🧰 Additional context used
🪛 LanguageTool
docs-v2/pages/components/contributing/guidelines.mdx
[style] ~412-~412: In American English, abbreviations like “etc.” require a period.
Context: ...et, encapsulate the API URL and token, etc). - The common.mjs` module contains lo...
(ETC_PERIOD)
[style] ~415-~415: In American English, abbreviations like “etc.” require a period.
Context: ... as version, dedupe, key, name, etc (those are specific to each component...
(ETC_PERIOD)
[style] ~422-~422: In American English, abbreviations like “etc.” require a period.
Context: ...s (e.g. name, description, key, etc) and potentially redefining any inherit...
(ETC_PERIOD)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
- GitHub Check: validate-links
🔇 Additional comments (6)
docs-v2/pages/components/contributing/guidelines.mdx (6)
119-121: Mandatory documentation link in action descriptions
Action components must include a documentation link using the[See the documentation](https://xxx)format as specified. This ensures users can quickly access relevant API docs.
164-166: Enforce version bump for affected components
New guideline to increment versions of all components importing or affected by an updated file is clear and aligns with semantic versioning best practices.
173-193: Standardize on.mjsfor all component files
All examples under Folder Structure have been updated to use.mjsextensions for ES modules (app, actions, sources). This aligns with the ES Modules requirement and prevents mixing CJS/ESM.
195-196: Require a dedicated/commonfolder for shared modules
The guideline now mandates placingcommon.mjs,utils.mjs, etc. inside a/commonfolder, improving discoverability and reuse.
410-424: Clarify Common Files (Optional) pattern
The expanded Common Files section clearly distinguishes between.app.mjs(API logic) andcommon.mjs(shared abstract logic), including inheritance/extension patterns and folder placement. This addition improves clarity for developers implementing shared modules.🧰 Tools
🪛 LanguageTool
[style] ~412-~412: In American English, abbreviations like “etc.” require a period.
Context: ...et, encapsulate the API URL and token, etc). - Thecommon.mjs` module contains lo...(ETC_PERIOD)
[style] ~415-~415: In American English, abbreviations like “etc.” require a period.
Context: ... asversion,dedupe,key,name, etc (those are specific to each component...(ETC_PERIOD)
[style] ~422-~422: In American English, abbreviations like “etc.” require a period.
Context: ...s (e.g.name,description,key, etc) and potentially redefining any inherit...(ETC_PERIOD)
440-441: Include all relevant API options as props
Updating the Props guideline to encourage exposing all pertinent API parameters as props increases component flexibility and reduces downstream prop additions.
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
docs-v2/pages/components/contributing/guidelines.mdx (1)
123-129:⚠️ Potential issueUpdate example snippet to include docs link
The sampleexport defaultsnippet still lacks the required[See the documentation](https://xxx)link in thedescription. Please update it to align with the new guideline:export default { key: "google_drive-new-shared-drive", name: "New Shared Drive", - description: "Emits a new event any time a shared drive is created.", + description: "Emits a new event any time a shared drive is created. [See the documentation](https://xxx)", version: "0.0.1", };
🧹 Nitpick comments (3)
docs-v2/pages/components/contributing/guidelines.mdx (3)
119-120: Clarify documentation link requirement
The new rule mandates that action component descriptions include a documentation link. It may be helpful to:
- Explicitly state whether this applies only to actions or also to sources once they support a
typefield.- Provide guidance on where to place the link (start, end, inline).
164-165: Clarify “affected” scope and add an example
The rule “increment the versions of all components that import or are affected by the updated file” is valuable but a bit vague. Consider adding:
- A short example (e.g., “If
common.mjschanges, bump the version in each component that imports it.”)- Definition of “affected” (e.g., direct imports vs. downstream dependencies)
411-412: Ensure “etc.” includes the period before closing parenthesis
American English abbreviations like “etc.” require a period. Please update each occurrence:- ...encapsulate the API URL and token, etc) + ...encapsulate the API URL and token, etc.) - ...attributes such as `version`, `dedupe`, `key`, `name`, etc (those are specific to each component) + ...attributes such as `version`, `dedupe`, `key`, `name`, etc. (those are specific to each component) - ...doesn't define attributes such as `version`, `dedupe`, `key`, `name`, etc (those are specific to each component) + ...doesn't define attributes such as `version`, `dedupe`, `key`, `name`, etc. (those are specific to each component)Also applies to: 415-415, 422-422
🧰 Tools
🪛 LanguageTool
[style] ~412-~412: In American English, abbreviations like “etc.” require a period.
Context: ...et, encapsulate the API URL and token, etc). - Thecommon.mjs` module contains lo...(ETC_PERIOD)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs-v2/pages/components/contributing/guidelines.mdx(8 hunks)
🧰 Additional context used
🪛 LanguageTool
docs-v2/pages/components/contributing/guidelines.mdx
[style] ~412-~412: In American English, abbreviations like “etc.” require a period.
Context: ...et, encapsulate the API URL and token, etc). - The common.mjs` module contains lo...
(ETC_PERIOD)
[style] ~415-~415: In American English, abbreviations like “etc.” require a period.
Context: ... as version, dedupe, key, name, etc (those are specific to each component...
(ETC_PERIOD)
[style] ~422-~422: In American English, abbreviations like “etc.” require a period.
Context: ...s (e.g. name, description, key, etc) and potentially redefining any inherit...
(ETC_PERIOD)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Lint Code Base
- GitHub Check: validate-links
🔇 Additional comments (4)
docs-v2/pages/components/contributing/guidelines.mdx (4)
175-181: Require.mjsextensions for ES modules
Updating the file extension guidance to use.mjsfor app, action, and source files correctly enforces the ES module requirement.
186-192: Update path examples to.mjs
The example paths ([app_slug].app.mjs,[action-name-slug].mjs, and[source-name-slug].mjs) now correctly reflect the ES module file extension changes.
440-441: Approve expanded prop inclusion guideline
Updating the props guidance to “incorporate all relevant options” (instead of limiting to 3–4) enhances flexibility and completeness.
790-793: Approve “Miscellaneous” section addition
Adding a “Miscellaneous” section with the camelCase naming rule is clear and reinforces consistency across props, methods, and variables.
Co-authored-by: Jorge Cortes <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
docs-v2/pages/components/contributing/guidelines.mdx (2)
195-196: Fix typo in “within”
This repeats a previously flagged typo. Please correct “withing” to “within.”- Common files (e.g., `common.mjs`, `utils.mjs`) must be placed withing a common + Common files (e.g., `common.mjs`, `utils.mjs`) must be placed within a common
423-424: Fix typo in “within” (again)
Another occurrence of “withing” here—please change it to “within.”- Common files (e.g., `common.mjs`, `utils.mjs`) must be placed withing a common + Common files (e.g., `common.mjs`, `utils.mjs`) must be placed within a common
🧹 Nitpick comments (2)
docs-v2/pages/components/contributing/guidelines.mdx (2)
119-120: Clarify the link‐formatting example
The backslashes before the square brackets may confuse readers. It’s clearer to wrap the entire literal Markdown example in backticks.
Apply this diff:- Action components require the description to include a link to the - relevant documentation in the following format: \[See the documentation\]\(https://public-api.com\) + Action components require the description to include a link to the + relevant documentation in the following format: `[See the documentation](https://public-api.com)`
164-165: Ensure consistent list formatting for the new version rule
The “increment versions” rule is currently a standalone paragraph but should align with the numbered steps above. Consider converting it into a numbered list item for consistency.
Example:- If you update a file, you must increment the versions of all components that - import or are affected by the updated file. + 4. If you update a file, increment the versions of all components that import it or are affected by it.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs-v2/pages/components/contributing/guidelines.mdx(8 hunks)
🧰 Additional context used
🪛 LanguageTool
docs-v2/pages/components/contributing/guidelines.mdx
[style] ~412-~412: In American English, abbreviations like “etc.” require a period.
Context: ...et, encapsulate the API URL and token, etc). - The common.mjs` module contains lo...
(ETC_PERIOD)
[style] ~415-~415: In American English, abbreviations like “etc.” require a period.
Context: ... as version, dedupe, key, name, etc (those are specific to each component...
(ETC_PERIOD)
[style] ~422-~422: In American English, abbreviations like “etc.” require a period.
Context: ...s (e.g. name, description, key, etc) and potentially redefining any inherit...
(ETC_PERIOD)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: validate-links
- GitHub Check: Lint Code Base
🔇 Additional comments (5)
docs-v2/pages/components/contributing/guidelines.mdx (5)
175-182: ESM file‐extension examples look good
Updating the sample folder tree to use.mjsfor ES modules matches the guidelines above.
186-193: Updated path examples are consistent
The changes renaming.jsfiles to.mjsin the sample component paths reflect the ESM requirement and look correct.
411-420: Great addition: clarifying the common‐module pattern
The new bullets explaining the role of.app.mjs,common.mjs, and component extensions provide valuable guidance on organizing shared logic.🧰 Tools
🪛 LanguageTool
[style] ~412-~412: In American English, abbreviations like “etc.” require a period.
Context: ...et, encapsulate the API URL and token, etc). - Thecommon.mjs` module contains lo...(ETC_PERIOD)
[style] ~415-~415: In American English, abbreviations like “etc.” require a period.
Context: ... asversion,dedupe,key,name, etc (those are specific to each component...(ETC_PERIOD)
440-441: Approve expanding props guidance
The new rule to include all relevant API options as props is clear and aligns with making components more flexible.
790-793: Approve new “Miscellaneous” section
Adding a reminder to use camelCase for props, methods, and variables helps enforce consistency across components.
michelle0927
moved this from Doing
to Ready for PR Review
in Component (Source and Action) Backlog
jcortes
moved this from Ready for PR Review
to Ready for Release
in Component (Source and Action) Backlog
github-project-automation
bot
moved this from Ready for Release
to Done
in Component (Source and Action) Backlog
Updating component guidelines to be more current.
Summary by CodeRabbit
.jsto.mjsand revised examples and folder structure accordingly./commonfolder.