-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Docs: move section on configuration variables #2910
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
Docs: move section on configuration variables #2910
Conversation
- to integration page documentation, as it is part of integration page documentation
📝 WalkthroughWalkthroughMoved configuration documentation out of Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Pre-merge checks✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to data retention organization setting 📒 Files selected for processing (1)
🔇 Additional comments (2)
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. Comment |
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/documenting/create-page.md (1)
117-117: Self-reference on line 117 is technically correct but slightly awkward.The link
(see [configuration variables details](documenting/create-page.md#configuration))points to the### Configurationsection (line 67) in the same file. This works, but a relative anchor (e.g.,#configuration) or descriptive text ("see the configuration section above") would improve readability.🔎 Suggested improvement for clarity
Replace the full path with a relative anchor:
- Configuration variables must document the accepted value types (see [configuration variables details](documenting/create-page.md#configuration)). + Configuration variables must document the accepted value types (see [configuration variables details](#configuration)).Alternatively, use descriptive text:
- Configuration variables must document the accepted value types (see [configuration variables details](documenting/create-page.md#configuration)). + Configuration variables must document the accepted value types (as described in the [Configuration](#configuration) section above).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (2)
docs/documenting/create-page.mddocs/documenting/standards.md
💤 Files with no reviewable changes (1)
- docs/documenting/standards.md
🔇 Additional comments (4)
docs/documenting/create-page.md (4)
69-94: Configuration section introduction and example are clear and well-structured.The updated introduction emphasizes the minimal required variables approach, and the expanded example with
api_key,name, andmonitored_conditionsprovides a practical template for integration authors. The use of the{% configuration %}tag and nestedkeys:for map types is well-demonstrated.
96-109: Type reference and available keys documentation are comprehensive.The type list (line 109) includes all necessary value types and multi-type handling (
[string, integer]), with clear cross-references to examples. The documentation ofrequired:andtype:keys provides sufficient detail for integration authors.
111-130: Verify that removed content from standards.md is fully captured and no broken references remain.The new "About configuration variables" subsection (lines 111-119) consolidates best-practice guidance previously distributed across standards.md, and the example block (lines 122-130) reinforces proper formatting. However, verify that:
- The standards.md file was updated to remove the "Configuration variables" and "UI variables" sections referenced in the PR summary.
- Any other pages that linked to those standards.md sections have been updated to point to this new location.
- Cross-references elsewhere in the documentation (e.g., line 144 in this file still references
documenting/standards.mdfor templating guidance) remain valid.
132-134: UI variables section is concise but accurate.The brief reference to
{% configuration_basic %}for UI variable documentation is appropriate, though readers may need to consult examples or integration pages to understand the distinction from YAML configuration variables. This is acceptable given the scope of this page.
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.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @docs/documenting/create-page.md:
- Line 109: The docs are inconsistent about multi-type notation for the `type:`
field; standardize on the YAML list form (e.g., `type: [string, integer]`)
throughout. Update the explanatory line that currently shows `string, integer`
to use the bracketed list form, and change any other occurrences to the same
`[type1, type2]` syntax so examples and text consistently reference the `type:`
field using YAML list notation.
🧹 Nitpick comments (1)
docs/documenting/create-page.md (1)
132-134: Expand the UI variables section with examples and clarification.The UI variables subsection is minimal (3 lines) and lacks guidance on:
- What UI variables are and how they differ from configuration variables
- When to use
{% configuration_basic %}versus{% configuration %}- Examples showing typical UI variable documentation
Consider adding an example block and clearer context, similar to the "About configuration variables" section.
🔎 Suggested expansion
### UI variables -- For describing **UI variables** use the `{% configuration_basic %}` section. +- For describing **UI variables** (variables exposed through the user interface configuration flow) use the `{% configuration_basic %}` section. +- UI variables are documented separately from YAML configuration variables and are used when the integration provides a config flow interface. + +#### Example UI variables block + +```yaml +{% configuration_basic %} +api_token: + description: Your API token for authentication. + required: true + type: string +{% endconfiguration_basic %} +```
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
docs/documenting/create-page.md
🔇 Additional comments (1)
docs/documenting/create-page.md (1)
111-131: Solid addition of configuration variable guidelines.The new "About configuration variables" subsection provides clear, actionable guidance for documentation authors, including YAML-only scope, tag requirements, and multi-type handling. The example block effectively illustrates the format. The inclusion of the link reference (line 117) addresses the prior review suggestion.
MartinHjelmare
left a comment
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.
Since it's a move we don't need to address the things I found, but wanted to point them out anyway.
Co-authored-by: Martin Hjelmare <marhje52@gmail.com>
Thank you for the catch! |
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.
Actionable comments posted: 0
♻️ Duplicate comments (1)
docs/documenting/create-page.md (1)
82-82: Inconsistent multi-type notation—standardize and add example.Line 82 advises using comma-separated notation (
string, integer), while line 132 advises using bracket notation ([string, integer]). These contradictory instructions must be resolved to provide clear guidance to contributors.Additionally, no example in the file demonstrates multi-type notation, making it impossible for contributors to verify the correct format and output.
🔎 Recommended fix: standardize on bracket notation and add an example
Update line 82 to match line 132's guidance:
- - For configuration variables that accept multiple types, separate the types with a comma (i.e. `string, integer`). + - For configuration variables that accept multiple types, use bracket notation (i.e. `[string, integer]`).Add a multi-type example to the YAML example block (lines 86–94):
some_key: description: This is a description of what this key is for. required: false type: string default: Optional default value - leave out if there isn't one +multi_type_key: + description: A key that accepts multiple types. + required: false + type: [string, integer]
🧹 Nitpick comments (1)
docs/documenting/create-page.md (1)
69-69: Clarify scope: YAML configuration vs. UI configuration.Line 69 states "Every integration page should contain a configuration sample" but doesn't clarify whether this refers to YAML, UI, or both. Given the context of the new subsections (UI variables vs. configuration variables for YAML), consider being more explicit.
-Every integration page should contain a configuration sample. +Every integration page should contain configuration examples. This includes YAML configuration for integrations that support it and/or UI variable descriptions for integrations with a configuration flow.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
docs/documenting/create-page.md
Proposed change
Docs: move section on configuration variables
Type of change
Checklist
Additional information
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.