1.. _docs-contributing: 2 3============ 4Contributing 5============ 6We'd love to accept your patches and contributions to Pigweed. There are just a 7few small guidelines you need to follow. 8 9Before participating in our community, please take a moment to review our 10:ref:`docs-code-of-conduct`. We expect everyone who interacts with the project 11to respect these guidelines. 12 13Get started 14----------- 15See :ref:`docs-get-started-upstream`. 16 17Pigweed contribution overview 18----------------------------- 19.. note:: 20 21 If you have any trouble with this flow, reach out in our `chat room 22 <https://discord.gg/M9NSeTA>`_ or on the `mailing list 23 <https://groups.google.com/forum/#!forum/pigweed>`_ for help. 24 25One-time contributor setup 26^^^^^^^^^^^^^^^^^^^^^^^^^^ 27#. Sign the 28 `Contributor License Agreement <https://cla.developers.google.com/>`_. 29#. Verify that your Git user email (git config user.email) is either Google 30 Account email or an Alternate email for the Google account used to sign 31 the CLA (Manage Google account → Personal Info → email). 32#. Obtain a login cookie from Gerrit's 33 `new-password <https://pigweed.googlesource.com/new-password>`_ page. 34#. Install the :ref:`gerrit-commit-hook` to automatically add a 35 ``Change-Id: ...`` line to your commit. 36#. Install the Pigweed presubmit check hook with ``pw presubmit --install``. 37 Remember to :ref:`activate-pigweed-environment` first! 38 39Presubmission process 40^^^^^^^^^^^^^^^^^^^^^ 41Before making or sending major changes or SEEDs, please reach out in our 42`chat room <https://discord.gg/M9NSeTA>`_ or on the `mailing list 43<https://groups.google.com/forum/#!forum/pigweed>`_ first to ensure the changes 44make sense for upstream. We generally go through a design phase before making 45large changes. See :ref:`SEED-0001` for a description of this process; but 46please discuss with us before writing a full SEED. Let us know of any 47priorities, timelines, requirements, and limitations ahead of time. 48 49For minor changes that don't fit the SEED process, follow the 50:ref:`docs-code_reviews-small-changes` guidance and the `Change submission 51process`_. 52 53.. warning:: 54 Skipping communicating with us before doing large amounts of work risks 55 accepting your contribution. Communication is key! 56 57Change submission process 58^^^^^^^^^^^^^^^^^^^^^^^^^ 59 60.. note:: 61 A change is a single git commit, and is also known as Change List or CL for 62 short. 63 64.. tip:: 65 Follow :ref:`docs-code_reviews-small-changes` for a smooth submission process 66 67#. Go through the `Presubmission process`_ and review this document's guidance. 68#. Ensure all files include the correct copyright and license headers. 69#. Include any necessary changes to the documentation. 70#. Run :ref:`module-pw_presubmit` to detect style or compilation issues before 71 uploading. 72#. Upload the change with ``git push origin HEAD:refs/for/main``. 73#. Add ``gwsq-pigweed@pigweed.google.com.iam.gserviceaccount.com`` as a 74 reviewer. This will automatically choose an appropriate person to review the 75 change. 76#. Address any reviewer feedback by amending the commit 77 (``git commit --amend``). 78#. Submit change to CI builders to merge. If you are not part of Pigweed's 79 core team, you can ask the reviewer to add the `+2 CQ` vote, which will 80 trigger a rebase and submit once the builders pass. 81 82Contributor License Agreement 83----------------------------- 84Contributions to this project must be accompanied by a Contributor License 85Agreement. You (or your employer) retain the copyright to your contribution; 86this simply gives us permission to use and redistribute your contributions as 87part of the project. Head over to <https://cla.developers.google.com/> to see 88your current agreements on file or to sign a new one. 89 90You generally only need to submit a CLA once, so if you've already submitted one 91(even if it was for a different project), you probably don't need to do it 92again. 93 94.. _docs-contributing-contribution-standards: 95 96Contribution Standards 97---------------------- 98Contributions, i.e. CLs, are expected to be complete solutions, accompanied by 99documentation and unit tests when merited, e.g. new code, bug fixes, or code 100that changes some behavior. This also means that code changes must be tested. 101Tests will avoid or minimize any negative impact to Pigweed users, while 102documentation will help others learn about the new changes. 103 104We understand that you may have different priorities or not know the best way to 105complete your contribution. In that case, reach out via our `chat room 106<https://discord.gg/M9NSeTA>`_ or on the `mailing list 107<https://groups.google.com/forum/#!forum/pigweed>`_ for help or `File a bug 108<https://issues.pigweed.dev/issues?q=status:open>`_ 109requesting fixes describing how this may be blocking you. Otherwise you risk 110working on a CL that does not match our vision and could be rejected. To keep 111our focus, we cannot adopt incomplete CLs. 112 113.. _gerrit-commit-hook: 114 115Build System Support 116-------------------- 117Pigweed users are split across a number of build systems including: 118 119* `Bazel <https://bazel.build/>`_ 120* `GN (a ninja generator) <https://gn.googlesource.com/gn/>`_ 121* `CMake <https://cmake.org/>`_ 122* `Soong (Android's build system) <https://source.android.com/docs/setup/build>`_ 123 124In order to ensure parity between different build systems, contributions must 125include support for at least Bazel (``BUILD.bazel``), GN (``BUILD.gn``) and 126CMake (``CMakeLists.txt``). 127 128We understand that most people don't have experience with all of the build 129systems Pigweed supports, and that this requirement is a burden on contributors. 130We find most people are able to follow the patterns in existing build files to 131add support for their changes; but not always. We're happy to help with build 132questions on Discord for that reason. Don't be shy if you need help! 133 134Gerrit Commit Hook 135------------------ 136Gerrit requires all changes to have a ``Change-Id`` tag at the bottom of each 137commit message. You should set this up to be done automatically using the 138instructions below. 139 140The commands below assume that your current working directory is the root 141of your Pigweed repository. 142 143**Linux/macOS** 144 145.. code-block:: bash 146 147 f=`git rev-parse --git-dir`/hooks/commit-msg ; mkdir -p $(dirname $f) ; curl -Lo $f https://gerrit-review.googlesource.com/tools/hooks/commit-msg ; chmod +x $f 148 149**Windows** 150 151.. code-block:: batch 152 153 git rev-parse --git-dir > gitrepopath.txt & set /p "g="< gitrepopath.txt & del gitrepopath.txt & call set "f=%g%/hooks" & call mkdir "%f%" & call curl -Lo "%f%/commit-msg" https://gerrit-review.googlesource.com/tools/hooks/commit-msg 154 155Commit Message 156-------------- 157See the :ref:`commit message section of the style 158guide<docs-pw-style-commit-message>` for how commit messages should look. 159 160Documentation 161------------- 162Most changes to Pigweed should have an associated documentation change. 163 164Building 165^^^^^^^^ 166To build the documentation, follow the :ref:`getting 167started<docs-get-started-upstream>` guide so you can build Pigweed. Then: 168 169#. Change to your checkout directory and ``. activate.sh`` if necessary 170#. Run ``pw watch -C out`` to build the code, run tests, and build docs 171#. Wait for the build to finish (see a ``PASS``) 172#. Navigate to ``<CHECKOUT>/out/docs/gen/docs/html/index.html`` 173#. Edit the relevant ``.rst`` file. Save when ready 174#. Refresh your browser after the build completes 175 176Alternately, you can use the local webserver in watch; this works better for 177some pages that have external resources: ``pw watch --serve-docs`` then 178navigate to `http://localhost:8000 <http://localhost:8000>`_ in your browser. 179 180Submission checklist 181^^^^^^^^^^^^^^^^^^^^ 182All Pigweed changes must either: 183 184#. Include updates to documentation, or 185#. Include ``No-Docs-Update-Reason: <reason>`` in a Gerrit comment on the CL. 186 For example: 187 188 * ``No-Docs-Update-Reason: formatting tweaks`` 189 * ``No-Docs-Update-Reason: internal cleanups`` 190 * ``No-Docs-Update-Reason: bugfix`` 191 192It's acceptable to only document new changes in an otherwise underdocumented 193module, but it's not acceptable to not document new changes because the module 194doesn't have any other documentation. 195 196Code Reviews 197------------ 198See :ref:`docs-code_reviews` for information about the code review process. 199 200Experimental Repository and Where to Land Code 201---------------------------------------------- 202Pigweed's has an `Experimental Repository 203<https://pigweed.googlesource.com/pigweed/experimental>`_ which differs from 204our main repository in a couple key ways: 205 206* Code is not expected to become production grade. 207* Code review standards are relaxed to allow experimentation. 208* In general the value of the code in the repository is the knowledge gained 209 from the experiment, not the code itself. 210 211Good uses of the repo include: 212 213* Experimenting with using an API (ex. C++20 coroutines) with no plans to 214 turn it into production code. 215* One-off test programs to gather data. 216 217We would like to avoid large pieces of code being developed in the experimental 218repository then imported into the Pigweed repository. If large amounts of code 219end up needing to migrate from experimental to main, then it must be landed 220incrementally as a series of reviewable patches, typically no 221`larger than 500 lines each 222<https://google.github.io/eng-practices/review/developer/small-cls.html>`_. 223This creates a large code review burden that often results in poorer reviews. 224Therefore, if the eventual location of the code will be the main Pigweed 225repository, it is **strongly encouraged** that the code be developed in the 226**main repository under an experimental flag**. 227 228.. note:: 229 The current organization of the experimental repository does not reflect 230 this policy. This will be re-organized once we have concrete recommendations 231 on its organization. 232 233Community Guidelines 234-------------------- 235This project follows `Google's Open Source Community Guidelines 236<https://opensource.google/conduct/>`_ and the :ref:`docs-code-of-conduct`. 237 238Presubmit Checks and Continuous Integration 239------------------------------------------- 240All Pigweed change lists (CLs) must adhere to Pigweed's style guide and pass a 241suite of automated builds, tests, and style checks to be merged upstream. Much 242of this checking is done using Pigweed's ``pw_presubmit`` module by automated 243builders. These builders run before each Pigweed CL is submitted and in our 244continuous integration infrastructure (see `Pigweed's build console 245<https://ci.chromium.org/p/pigweed/g/pigweed/console>`_). 246 247Running Presubmit Checks 248^^^^^^^^^^^^^^^^^^^^^^^^ 249To run automated presubmit checks on a pending CL, click the ``CQ DRY RUN`` 250button in the Gerrit UI. The results appear in the Tryjobs section, below the 251source listing. Jobs that passed are green; jobs that failed are red. 252 253If all checks pass, you will see a ``Dry run: This CL passed the CQ dry run.`` 254comment on your change. If any checks fail, you will see a ``Dry run: Failed 255builds:`` message. All failures must be addressed before submitting. 256 257In addition to the publicly visible presubmit checks, Pigweed runs internal 258presubmit checks that are only visible within Google. If any these checks fail, 259external developers will see a ``Dry run: Failed builds:`` comment on the CL, 260even if all visible checks passed. Reach out to the Pigweed team for help 261addressing these issues. 262 263Project Presubmit Checks 264^^^^^^^^^^^^^^^^^^^^^^^^ 265In addition to Pigweed's presubmit checks, some projects that use Pigweed run 266their presubmit checks in Pigweed's infrastructure. This supports a development 267flow where projects automatically update their Pigweed submodule if their tests 268pass. If a project cannot build against Pigweed's tip-of-tree, it will stay on 269a fixed Pigweed revision until the issues are fixed. See the `examples 270<https://pigweed.googlesource.com/pigweed/examples/>`_ repo for an example of 271this. 272 273Pigweed does its best to keep builds passing for dependent projects. In some 274circumstances, the Pigweed maintainers may choose to merge changes that break 275dependent projects. This will only be done if 276 277* a feature or fix is needed urgently in Pigweed or for a different project, 278 and 279* the project broken by the change does not imminently need Pigweed updates. 280 281The downstream project will continue to build against their last working 282revision of Pigweed until the incompatibilities are fixed. 283 284In these situations, Pigweed's commit queue submission process will fail for all 285changes. If a change passes all presubmit checks except for known failures, the 286Pigweed team may permit manual submission of the CL. Contact the Pigweed team 287for submission approval. 288 289Code coverage in Gerrit 290^^^^^^^^^^^^^^^^^^^^^^^ 291Unit test coverage data for C++ is computed by the ``coverage`` builder and 292displayed in Gerrit. 293 294.. image:: https://storage.googleapis.com/pigweed-media/gerrit_code_coverage.png 295 :width: 800 296 :alt: Code coverage display in Gerrit 297 298#. **When will coverage data be visible on my CL?** The coverage builder needs 299 to finish running (about 6 minutes), and then the data needs to be ingested 300 by the coverage pipeline (ran every 30 minutes). 301 302#. **What tests is this based on?** Only the C++ unit tests of the modules ran 303 as part of the GN build. (There's no coverage data for Python or Rust yet.) 304 305#. **Can I generate a coverage report locally?** Yes. Running ``pw 306 presubmit --step coverage`` will generate a HTML report at 307 ``out/presubmit/coverage/host_clang_coverage/obj/coverage_report/html/index.html``. 308 309#. **I'd love to have this in my Pigweed-based project!** See 310 :ref:`module-pw_build-gn-pw_coverage_report` for GN and 311 :ref:`docs-build_system-bazel_coverage` for Bazel. 312 313Running local presubmits 314------------------------ 315To speed up the review process, consider adding :ref:`module-pw_presubmit` as a 316git push hook using the following command: 317 318Linux/macOS 319^^^^^^^^^^^ 320.. code-block:: bash 321 322 $ pw presubmit --install 323 324This will be effectively the same as running the following command before every 325``git push``: 326 327.. code-block:: bash 328 329 $ pw presubmit 330 331 332.. image:: ../../pw_presubmit/docs/pw_presubmit_demo.gif 333 :width: 800 334 :alt: pw presubmit demo 335 336If you ever need to bypass the presubmit hook (due to it being broken, for 337example) you may push using this command: 338 339.. code-block:: bash 340 341 $ git push origin HEAD:refs/for/main --no-verify 342 343Presubmit and branch management 344^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 345When creating new feature branches, make sure to specify the upstream branch to 346track, e.g. 347 348.. code-block:: bash 349 350 $ git checkout -b myfeature origin/main 351 352When tracking an upstream branch, ``pw presubmit`` will only run checks on the 353modified files, rather than the entire repository. 354 355Presubmit flags 356^^^^^^^^^^^^^^^ 357``pw presubmit`` can accept a number of flags 358 359``-b commit, --base commit`` 360 Git revision against which to diff for changed files. Default is the tracking 361 branch of the current branch. Set commit to "HEAD" to check files added or 362 modified but not yet commited. Cannot be used with --full. 363 364``--full`` 365 Run presubmit on all files, not just changed files. Cannot be used with 366 --base. 367 368``-e regular_expression, --exclude regular_expression`` 369 Exclude paths matching any of these regular expressions, which are interpreted 370 relative to each Git repository's root. 371 372``-k, --keep-going`` 373 Continue instead of aborting when errors occur. 374 375``--output-directory OUTPUT_DIRECTORY`` 376 Output directory (default: <repo root>/out/presubmit) 377 378``--package-root PACKAGE_ROOT`` 379 Package root directory (default: <output directory>/packages) 380 381``--clear, --clean`` 382 Delete the presubmit output directory and exit. 383 384``-p, --program PROGRAM`` 385 Which presubmit program to run 386 387``--step STEP`` 388 Provide explicit steps instead of running a predefined program. 389 390``--install`` 391 Install the presubmit as a Git pre-push hook and exit. 392 393.. _Sphinx: https://www.sphinx-doc.org/ 394 395.. inclusive-language: disable 396 397.. _reStructuredText Primer: https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html 398 399.. inclusive-language: enable 400 401.. _docs-contributing-presubmit-virtualenv-hashes: 402 403Updating Python dependencies in the virtualenv_setup directory 404^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 405If you update any of the requirements or constraints files in 406``//pw_env_setup/py/pw_env_setup/virtualenv_setup``, you must run this command 407to ensure that all of the hashes are updated: 408 409.. code-block:: console 410 411 pw presubmit --step update_upstream_python_constraints --full 412 413For Python packages that have native extensions, the command needs to be run 3 414times: once on Linux, once on macOS, and once on Windows. Please run it on the 415OSes that are available to you; a core Pigweed teammate will run it on the rest. 416See the warning about caching Python packages for multiple platforms in 417:ref:`docs-python-build-downloading-packages`. 418 419.. toctree:: 420 :maxdepth: 1 421 :hidden: 422 423 ../embedded_cpp_guide 424 ../style_guide 425 ../code_reviews 426 ../code_of_conduct 427 docs/index 428