Update contributing.rst #4188
Update contributing.rst #4188
Conversation
WalkthroughThe recent updates to the contributing guidelines for the ESPHome project include enhancements to clarity and consistency. Changes encompass the use of more inclusive language, expanded instructions for documentation contributions, and clearer expectations for pull request submissions. The guidelines also outline standards for code quality and validation, ensuring that contributors have a comprehensive understanding of the process and requirements for successful participation. Changes
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
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 Configuration File (
|
✅ Deploy Preview for esphome ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
|
||
| This method is also automatically loaded and invoked by the ESPHome core. An example of | ||
| such a method can be seen below: | ||
| This method is also automatically loaded and invoked by the ESPHome core. Here's an example of such a method: | ||
|
|
||
| .. code-block:: python |
There was a problem hiding this comment.
Can we update this block to use async/await and also update the text below to remove all references to yield and use await.
There was a problem hiding this comment.
See how this looks: 33d4838 (#4188)
guides/contributing.rst
Outdated
| .. note:: | ||
|
|
||
| **Code in** ``loop()``, ``update()`` **and** ``setup()`` **must not block**. Specifically, methods like ``delay()`` | ||
| should be avoided and **delays longer than 10 ms are not permitted**. The reason for this is that ESPHome uses a | ||
| single-threaded loop for all components -- if your component blocks, it will delay the whole loop, negatively | ||
| impacting other components. This can result in a variety of problems such as network connections being lost. |
There was a problem hiding this comment.
Maybe here give reference to this->set_timeout or using a state-machine to handle "waiting" in loop for the state to become what is needed for the next stage?
There was a problem hiding this comment.
See how this looks: 33d4838 (#4188)
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
Co-authored-by: Jesse Hills <3060199+jesserockz@users.noreply.github.com>
Co-authored-by: Jesse Hills <3060199+jesserockz@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Co-authored-by: Jesse Hills <3060199+jesserockz@users.noreply.github.com>
| @@ -697,14 +747,28 @@ The next method that will be called with your component is ``loop()`` (or ``upda | |||
| :apiclass:`PollingComponent`). These methods should retrieve the latest data from your component and publish them with | |||
| the provided methods. | |||
|
|
|||
| .. note:: | |||
| Finally, your component must have a ``dump_config`` method that prints the complete user configuration. | |||
There was a problem hiding this comment.
when dump_config is an required method to be override. would it not good idea to change up the Component class by changing "Virtual dump_config() {}" into "Virtual dump_config() = 0;"?
Description:
Various updates, particularly around PR submission. Lots of other tweaks & copy fixes/corrections.
Related issue (if applicable): fixes
Pull request in esphome with YAML changes (if applicable): esphome/esphome#
Checklist:
I am merging into
nextbecause this is new documentation that has a matching pull-request in esphome as linked above.or
I am merging into
currentbecause this is a fix, change and/or adjustment in the current documentation and is not for a new component or feature.Link added in
/index.rstwhen creating new documents for new components or cookbook.