-
-
Notifications
You must be signed in to change notification settings - Fork 7.7k
Fixed the cmps abbreviation #39757
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
Fixed the cmps abbreviation #39757
Conversation
The abbreviation is listed as `cmp`, but it should be `cmps`. The examples are OK, just the text description above example is not.
✅ Deploy Preview for home-assistant-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThe documentation for MQTT device discovery was updated to correct the abbreviation for the key holding component-specific options, changing it from Changes
Sequence Diagram(s)No sequence diagram generated as the changes are limited to documentation updates. 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 (
|
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
📜 Review details
Configuration used: CodeRabbit 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)
source/_integrations/mqtt.markdown
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`source/_integrations/*`: Integration documentation must use the provided template structure and section order.
source/_integrations/*
: Integration documentation must use the provided template structure and section order.
📄 Source: CodeRabbit Inference Engine (.github/copilot-instructions.md)
List of files the instruction was applied to:
source/_integrations/mqtt.markdown
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Redirect rules - home-assistant-docs
- GitHub Check: Header rules - home-assistant-docs
- GitHub Check: Pages changed - home-assistant-docs
The component specific options are placed as mappings under the `components` key (abbreviated as `cmps`) like: | ||
|
||
```json | ||
{ |
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.
🛠️ Refactor suggestion
Abbreviation updated here but still inconsistent further down
Line 373 correctly switches the abbreviation to cmps
, yet several subsequent references (e.g. line 412 & line 572) still mention cmp
. Readers will be confused by mixed usage.
Please align the entire document to cmps
, e.g.:
- The components id's under the `components` (`cmp`) key, are used as part of the discovery identification.
+ The component IDs under the `components` (`cmps`) key are used as part of the discovery identification.
and
- Discovery id: `0AFFD2 bla` *(`0AFFD2`from discovery topic, `bla`: The key under `cmp` in the discovery payload)*
+ Discovery id: `0AFFD2 bla` *(`0AFFD2` from discovery topic, `bla`: The key under `cmps` in the discovery payload)*
Ensuring a single, correct abbreviation prevents implementation errors in user payloads.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In source/_integrations/mqtt.markdown around lines 373 to 376 and further down
near lines 412 and 572, the abbreviation for the components key is
inconsistently used as both `cmps` and `cmp`. To fix this, update all
occurrences of `cmp` to `cmps` throughout the entire document to maintain
consistency and prevent confusion or implementation errors.
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.
Addressed in #39762
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.
@jbouwh Thank you for addressing the inconsistency! It's great that you created PR #39762 to fix all the mixed usage of cmp
vs cmps
throughout the document. This will prevent confusion for users implementing MQTT discovery payloads and ensure consistent documentation.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
The abbreviation is listed as
cmp
, but it should becmps
. The examples are OK, just the text description above example is not.Proposed change
Just a small change to fix the docs - the descriptive text is missing letter s from cmps abbreviation explanation.
Type of change
current
branch).current
branch).next
branch).next
branch).Additional information
Checklist
current
branch.next
branch.Summary by CodeRabbit