diff --git a/guides/contributing.rst b/guides/contributing.rst index d9c52d1d9f..7b5c068ea4 100644 --- a/guides/contributing.rst +++ b/guides/contributing.rst @@ -546,9 +546,6 @@ wish to use it. Now you can open ESPHome in your IDE of choice (many of us are using `VSCode `__) with the PlatformIO addons (see PlatformIO docs for more info) and develop the new feature with the guidelines below. -All PRs are automatically checked and tested for some common formatting/code errors with Github Actions. *These checks* -**must all pass** *before we will review (and eventually merge) your PR.* - Setting Up Git Environment -------------------------- @@ -597,7 +594,7 @@ near the top of the page (or, alternatively, go to branches and create it from t - Complete the Pull Request template: - Include a brief (but complete) summary of your changes. - - PRs without a descrption/summary of the changes will not be reviewed or merged, although exceptions may + - PRs without a description/summary of the changes will not be reviewed or merged, although exceptions may occasionally be made for small PRs and/or PRs made by frequent contributors/maintainers. - **Do not delete the template.** @@ -611,15 +608,47 @@ near the top of the page (or, alternatively, go to branches and create it from t it is ready. We do this because, if a PR is reviewed and then it changes, it must be re-reviewed. Reviewing a single PR multiple times is not a productive use of time and we try as much as possible to avoid doing so. +So now that you've created your PR...you're not quite done! Read on to the next section below so you know what to +expect next. + Review Process ************** -ESPHome's maintainers work hard to maintain a high standard for its code, so reviews can take some time. At the bottom -of each pull request you will see the "Github Actions" continuous integration (CI) checks which will automatically -analyize all code changed in your branch. These checks try to spot (and suggest corrections for) errors. If any CI -check fails, please look at the Github Actions log and fix all errors that appear there. +.. _automated_checks: + +Automated Checks +^^^^^^^^^^^^^^^^ + +At the bottom of each pull request you will see the "Github Actions" continuous integration (CI) checks which will +automatically analyze all code changed in your branch. These checks try to spot (and suggest corrections for) common +errors; they look like this: + +.. figure:: images/gha_checks.jpg + :align: center + :width: 100.0% + :alt: Automated checks on PR by GitHub Actions + :class: light-invert + +You can click the "Details" link to the right of each check to see the logs for that check. If a red ❌ appears next to +any given check, *you'll need to view that check's logs and make the suggested changes so that the test will pass.* + +**Implementing Feedback from Automated Checks** + +Occasionally, an automated check may suggest a change that either isn't directly related to your PR or that may require +changes to other components/platforms. When this happens, please create a new/additional PR to implement this change. + +For example, the automated checks may suggest moving a constant from your (new) component/platform into ``const.py``. +This is a simple change, but we require that it is done in a separate PR. + +Ultimately, **all automated checks must be passing** before maintainers will review and (eventually) merge your PR! + +Review by Maintainers +^^^^^^^^^^^^^^^^^^^^^ + +ESPHome's maintainers work hard to maintain a high standard for its code, so reviews by a human can take some time. -**All automated checks must be passing** before a given PR will be reviewed and (eventually) merged! +**All automated checks must be passing** before maintainers will review and (eventually) merge your PR! See +:ref:`automated_checks` above. **When will my PR be reviewed/merged?** @@ -709,7 +738,7 @@ Beyond basic functionality (*"does it work?"*), here are a few other items we ch .. _prs-are-being-drafted-when-changes-are-needed: Why was my PR marked as a draft? -************************************ +******************************** If your PR was reviewed and changes were requested, our bot will automatically mark your PR as a draft. This means that the PR is not ready to be merged or further reviewed for the moment. @@ -1006,7 +1035,7 @@ it then attempts to read back the measurement from the sensor. :apiclass:`PollingComponent`. For any :apiclass:`Component` (which is nearly everything), the well-known ``set_timeout`` method is also available; -this can be a handy alternative to implemeting a state machine. +this can be a handy alternative to implementing a state machine. .. _a_note_about_custom_components: @@ -1124,8 +1153,8 @@ ESPHome's maintainers work hard to maintain a high standard for its code. We try - Components should dump their configuration using ``ESP_LOGCONFIG`` at startup in ``dump_config()``. - ESPHome uses a unified formatting tool for all source files (but this tool can be difficult to install). - When creating a new PR in GitHub, be sure to check the Github Actions output to see what formatting needs to be - changed and what potential problems are detected. + When creating a new PR in GitHub, be sure to check the :ref:`Github Actions` output to see what + formatting needs to be changed and what potential problems are detected. - Use of external libraries should be kept to a minimum: - If the component you're developing has a simple communication interface, please consider implementing the library @@ -1135,7 +1164,7 @@ ESPHome's maintainers work hard to maintain a high standard for its code. We try communication abstractions. - If the library accesses a global variable/state (``Wire`` is a good example) then there's likely a problem because the component may not be modular. Put another way, this approach may mean that it's not possible to create multiple - instances of the component for use wihtin ESPHome. + instances of the component for use within ESPHome. - Components **must** use the provided abstractions like ``sensor``, ``switch``, etc. Components specifically should **not** directly access other components -- for example, to publish to MQTT topics. diff --git a/guides/images/gha_checks.jpg b/guides/images/gha_checks.jpg new file mode 100644 index 0000000000..e5200200fb Binary files /dev/null and b/guides/images/gha_checks.jpg differ