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