Skip to content

Commit

Permalink
Add GHA detail, copy fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
kbx81 committed Dec 10, 2024
1 parent 0224cf8 commit 2da1c57
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 14 deletions.
57 changes: 43 additions & 14 deletions guides/contributing.rst
Original file line number Diff line number Diff line change
Expand Up @@ -546,9 +546,6 @@ wish to use it.
Now you can open ESPHome in your IDE of choice (many of us are using `VSCode <https://code.visualstudio.com/download>`__)
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
--------------------------

Expand Down Expand Up @@ -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.**

Expand All @@ -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?**

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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:

Expand Down Expand Up @@ -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<automated_checks>` 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
Expand All @@ -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.
Expand Down
Binary file added guides/images/gha_checks.jpg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit 2da1c57

Please sign in to comment.