From 002e049fa91335730ee98e6f7d67779cbac4c33c Mon Sep 17 00:00:00 2001 From: driazati <9407960+driazati@users.noreply.github.com> Date: Wed, 6 Apr 2022 09:54:04 -0700 Subject: [PATCH] [docs][ci] Add CI reproducability docs (#10912) This adds info about `ci.py` to the docs and also re-arranges things a bit to consolidate/de-duplicate information Co-authored-by: driazati <driazati@users.noreply.github.com> --- docs/contribute/ci.rst | 8 +- docs/contribute/code_guide.rst | 16 +++- docs/contribute/code_review.rst | 21 ++---- docs/contribute/committer_guide.rst | 5 ++ docs/contribute/community.rst | 5 +- docs/contribute/document.rst | 8 +- docs/contribute/error_handling.rst | 5 ++ docs/contribute/git_howto.rst | 4 + docs/contribute/index.rst | 6 +- docs/contribute/pull_request.rst | 109 +++++++++++++++++----------- docs/contribute/release_process.rst | 8 +- 11 files changed, 121 insertions(+), 74 deletions(-) diff --git a/docs/contribute/ci.rst b/docs/contribute/ci.rst index 0fdab3f925701..d40e4d5ab74b2 100644 --- a/docs/contribute/ci.rst +++ b/docs/contribute/ci.rst @@ -20,6 +20,9 @@ Using TVM's CI ============== +.. contents:: + :local: + TVM uses Jenkins for running Linux continuous integration (CI) tests on `branches <https://ci.tlcpack.ai/job/tvm/>`_ and `pull requests <https://ci.tlcpack.ai/job/tvm/view/change-requests/>`_ through a @@ -58,10 +61,7 @@ the failing job to view the logs. Note: Reproduce Failures ------------------ -Most TVM Python tests run under |pytest|_ and -can be run as described in :ref:`pr-testing`. For a closer environment to the one -than runs in CI you can run the docker images directly, build TVM, and execute -tests inside the container. See :ref:`docker_images` for details. +Most TVM Python tests run under |pytest|_ and can be run as described in :ref:`pr-testing`. Keeping CI Green **************** diff --git a/docs/contribute/code_guide.rst b/docs/contribute/code_guide.rst index 725c3ce67b281..a7137297f1868 100644 --- a/docs/contribute/code_guide.rst +++ b/docs/contribute/code_guide.rst @@ -20,6 +20,10 @@ Code Guide and Tips =================== +.. contents:: + :depth: 2 + :local: + This is a document used to record tips in TVM codebase for reviewers and contributors. Most of them are summarized through lessons during the contributing and process. @@ -34,14 +38,18 @@ C++ Code Styles pass by value is better than pass by const reference in such cases. - Favor ``const`` member function when possible. -We use `clang-format` to enforce the code style. Because different version +We use ``clang-format`` to enforce the code style. Because different version of clang-format might change by its version, it is recommended to use the same version of the clang-format as the main one. You can also use the following command via docker. .. code:: bash - docker/bash.sh tlcpack/ci-lint clang-format-10 [path-to-file] + # Run a specific file through clang-format + docker/bash.sh ci_lint clang-format-10 [path-to-file] + + # Run all linters, including clang-format + python tests/scripts/ci.py lint clang-format is also not perfect, when necessary, you can use disble clang-format on certain code regions. @@ -78,8 +86,8 @@ Because clang-format may not recognize macros, it is recommended to use macro li Python Code Styles ------------------ - The functions and classes are documented in `numpydoc <https://numpydoc.readthedocs.io/en/latest/>`_ format. -- Check your code style using ``make pylint`` -- Stick to language features as in ``python 3.6`` +- Check your code style using ``python tests/scripts/ci.py lint`` +- Stick to language features in ``python 3.7`` Writing Python Tests diff --git a/docs/contribute/code_review.rst b/docs/contribute/code_review.rst index 173f8577ab37c..48bda114e9cb4 100644 --- a/docs/contribute/code_review.rst +++ b/docs/contribute/code_review.rst @@ -18,9 +18,12 @@ .. _code_review_guide: -Perform Code Reviews -==================== +Code Reviews +============ +.. contents:: + :depth: 2 + :local: Open source code is maintained by a community with diverse backgrounds, interests, and goals. Hence it is important to provide clear, documented and maintainable code and processes. Code reviews are a @@ -152,18 +155,6 @@ Our goal is to strive to be consistent and objective but all of us are unfortuna Additional Recommendations -------------------------- -Scope the PRs -~~~~~~~~~~~~~ - -We recommend authors to send well scoped PRs that are easy to review and revert in case there is a problem. -Authors avoid merging multiple unrelated changes into a single PR and split them into separate PRs. - -Label the PRs with Area Prefix -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -When sending pull requests, it is helpful to prefix the PR title with the areas related PR(e.g. use [TIR] for TIR-related changes). -This would help people recognize the related areas and find PRs they are interested in. - - Deliberate on API and Data Structures ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ A minimum and stable API is critical to the project’s life. A good API makes a huge difference. Always think very carefully about all the aspects including naming, argument definitions and behavior. @@ -193,7 +184,7 @@ Minimize Dependencies ~~~~~~~~~~~~~~~~~~~~~ Always be cautious in introducing dependencies. While it is important to reuse code and avoid reinventing the wheel, dependencies can increase burden of users in deployment. A good design principle is that a feature or function -should only have a dependecy if/when a user actually use it. +should only have a dependency if/when a user actually use it. Concise Implementation ~~~~~~~~~~~~~~~~~~~~~~ diff --git a/docs/contribute/committer_guide.rst b/docs/contribute/committer_guide.rst index 3dc5bf07f3cdd..d0924400543ed 100644 --- a/docs/contribute/committer_guide.rst +++ b/docs/contribute/committer_guide.rst @@ -19,6 +19,11 @@ Committer Guide =============== + +.. contents:: + :depth: 2 + :local: + This is an evolving document to provide some helpful tips for committers. Most of them are lessons learned during development. We welcome every committer to contribute to this document. diff --git a/docs/contribute/community.rst b/docs/contribute/community.rst index c41c7f394dd50..2e21d372eed3e 100644 --- a/docs/contribute/community.rst +++ b/docs/contribute/community.rst @@ -20,9 +20,12 @@ TVM Community Guidelines ======================== -TVM adopts the Apache style model and governs by merit. We believe that it is important to create an inclusive community where everyone can use, contribute to, and influence the direction of the project. See `CONTRIBUTORS.md <https://github.com/apache/tvm/blob/main/CONTRIBUTORS.md>`_ for the current list of contributors. +.. contents:: + :depth: 2 + :local: +TVM adopts the Apache style model and governs by merit. We believe that it is important to create an inclusive community where everyone can use, contribute to, and influence the direction of the project. See `CONTRIBUTORS.md <https://github.com/apache/tvm/blob/main/CONTRIBUTORS.md>`_ for the current list of contributors. General Development Process --------------------------- diff --git a/docs/contribute/document.rst b/docs/contribute/document.rst index ffd63490ddf42..43f98ded74017 100644 --- a/docs/contribute/document.rst +++ b/docs/contribute/document.rst @@ -17,8 +17,12 @@ .. _doc_guide: -Write Documentation for TVM -=========================== +Documentation +============= + +.. contents:: + :depth: 2 + :local: TVM documentation loosely follows the `formal documentation style described by Divio <https://documentation.divio.com>`_. This system has been chosen because diff --git a/docs/contribute/error_handling.rst b/docs/contribute/error_handling.rst index d31b401ea6540..ee5f0c100e4b0 100644 --- a/docs/contribute/error_handling.rst +++ b/docs/contribute/error_handling.rst @@ -19,6 +19,11 @@ Error Handling Guide ==================== + +.. contents:: + :depth: 2 + :local: + TVM contains structured error classes to indicate specific types of error. Please raise a specific error type when possible, so that users can write code to handle a specific error category if necessary. diff --git a/docs/contribute/git_howto.rst b/docs/contribute/git_howto.rst index 1271aad8a2684..ca12f6fddbed4 100644 --- a/docs/contribute/git_howto.rst +++ b/docs/contribute/git_howto.rst @@ -21,6 +21,10 @@ Git Usage Tips ============== +.. contents:: + :depth: 2 + :local: + Here are some tips for git workflow. How to resolve a conflict with ``main`` diff --git a/docs/contribute/index.rst b/docs/contribute/index.rst index aa893dbccb72b..d30dd3e8b0b4e 100644 --- a/docs/contribute/index.rst +++ b/docs/contribute/index.rst @@ -41,12 +41,12 @@ Here are guidelines for contributing to various aspect of the project: :maxdepth: 2 community + pull_request code_review committer_guide document code_guide - error_handling - pull_request git_howto ci - release_process \ No newline at end of file + release_process + error_handling diff --git a/docs/contribute/pull_request.rst b/docs/contribute/pull_request.rst index 226e693e2c724..82b5c5d43f416 100644 --- a/docs/contribute/pull_request.rst +++ b/docs/contribute/pull_request.rst @@ -18,9 +18,16 @@ Submit a Pull Request ===================== -This is a quick guide to submit a pull request, please also refer to the detailed guidelines. +.. contents:: + :depth: 2 + :local: -- Before submit, please rebase your code on the most recent version of main, you can do it by +Guidelines +---------- + +- We recommend authors send well scoped PRs that are easy to review and revert in case there is a problem. As such, authors should avoid merging multiple unrelated changes into a single PR +- Before you submit a PR, please rebase your code on the most recent version of ``main``, you can do it by + running .. code:: bash @@ -28,33 +35,34 @@ This is a quick guide to submit a pull request, please also refer to the detaile git fetch upstream git rebase upstream/main -- Make sure code style check pass by typing the following command, and all the existing test-cases pass. +- Make sure code passes lint checks - .. code:: bash + .. code:: bash - # Run all lint steps. - docker/lint.sh + # While the lint commands used should be identical to those run in CI, this command reproduces + # the CI lint procedure exactly (typically helpful for debugging lint script errors or + # to avoid installing tools manually) + python tests/scripts/ci.py lint - # To run steps individually, specify their step names on the command-line. An incorrectly - # spelled step name causes the tool to print all available steps. - docker/lint.sh <step_name> ... + # Run all lint steps. + docker/lint.sh - # While the lint commands used should be identical to those run in CI, this command reproduces - # the CI lint procedure exactly (typically helpful for debugging lint script errors). - docker/bash.sh ci_lint ./tests/scripts/task_lint.sh + # To run steps individually, specify their step names on the command-line. An incorrectly + # spelled step name causes the tool to print all available steps. + docker/lint.sh <step_name> ... - When the clang-format lint check fails, run git-clang-format as follows to automatically reformat - your code: + If the clang-format lint check fails, run git-clang-format as follows to automatically reformat + your code: - .. code:: bash + .. code:: bash - # Run clang-format check for all the files that changed since upstream/main - docker/bash.sh ci_lint ./tests/lint/git-clang-format.sh upstream/main + # Run clang-format check for all the files that changed since upstream/main + docker/bash.sh ci_lint ./tests/lint/git-clang-format.sh upstream/main - Add test-cases to cover the new features or bugfix the patch introduces. - Document the code you wrote, see more at :ref:`doc_guide` -- Send the pull request and fix the problems reported by automatic checks. -- Request code reviews from other contributors and improves your patch according to feedbacks. +- `Create a pull request <https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request>`_ and fix the problems reported by CI checks. +- Request code reviews from other contributors and improve your patch according to their reviews by ``@``-ing them in your pull request. Tags in PR titles will automatically tag subscribed users, so make sure to put relevant topics in your PR titles (e.g. ``[microTVM] a cool change`` and not ``a cool change for microTVM``). - To get your code reviewed quickly, we encourage you to help review others' code so they can do the favor in return. - Code review is a shepherding process that helps to improve contributor's code quality. @@ -62,29 +70,13 @@ This is a quick guide to submit a pull request, please also refer to the detaile We highly value patches that can get in without extensive reviews. - The detailed guidelines and summarizes useful lessons. -- The patch can be merged after the reviewers approve the pull request. - - +- The PR can be merged after the reviewers approve the pull request. CI Environment -------------- -We use docker container to create stable CI environments -that can be deployed to multiple machines. -Because we want a relatively stable CI environment and make use of pre-cached image, -all of the CI images are built and maintained by committers. - -Upgrade of CI images can cause problems and need fixes to accommodate the new env. -Here is the protocol to update CI image: - -- Send PR to upgrade build script in the repo - - Can be done by a contributor, the following steps need committership. -- Build the new docker image -- Tag the docker image with a new version and push to tvmai -- Update the version(most of the time increase the minor version) in the Jenkinsfile, send a PR. -- Fix any issues wrt to the new image versions in the PR. -- Merge the PR and now we are in new version. -- Tag the new version as the latest. -- Periodically cleanup the old versions on local workers +We use Docker images to create stable CI environments that can be deployed to multiple machines. +Follow the steps in `this issue template <https://github.com/apache/tvm/issues/new?assignees=&labels=&template=ci-image.md&title=%5BCI+Image%5D+>`_ +to update a CI Docker image. .. _pr-testing: @@ -93,11 +85,42 @@ Testing Even though we have hooks to run unit tests automatically for each pull request, it's always recommended to run unit tests locally beforehand to reduce reviewers' burden and speedup review process. +Docker (recommended) +^^^^^^^^^^^^^^^^^^^^ +``tests/scripts/ci.py`` replicates the CI environment locally and provides a user-friendly interface. +The same Docker images and scripts used in CI are used directly to run tests. It also deposits builds +in different folders so you can maintain multiple test environments without rebuilding from scratch +each time (e.g. you can test a change in CPU and i386 while retaining incremental rebuilds). + +.. code:: bash + + # see all available platforms + python tests/scripts/ci.py --help + python tests/scripts/ci.py cpu --help + + # run the CPU build in the ci_cpu docker container (build will be left in + # the build-cpu/ folder) + # note: the CPU and GPU Docker images are quite large and may take some + # time to download on their first use + python tests/scripts/ci.py cpu + + # run the CPU build in the ci_cpu docker container and then run unittests + python tests/scripts/ci.py cpu --unittest + + # quickly iterate by running a specific test and skipping the rebuild each time + python tests/scripts/ci.py cpu --skip-build --tests tests/python/unittest/test_tir_transform_inject_rolling_buffer.py::test_upscale + + # run the CPU build and drop into a shell in the container + python tests/scripts/ci.py cpu --interactive + + +C++ (local) +^^^^^^^^^^^ + Running the C++ tests requires installation of gtest, following the instructions in :ref:`install-from-source-cpp-tests` -C++ -^^^ + .. code:: bash # assume you are in tvm source root @@ -105,8 +128,8 @@ C++ ./tests/scripts/task_cpp_unittest.sh -Python -^^^^^^ +Python (local) +^^^^^^^^^^^^^^ Necessary dependencies: .. code:: bash diff --git a/docs/contribute/release_process.rst b/docs/contribute/release_process.rst index f330a7ddd3e61..e2bf6455b5af8 100644 --- a/docs/contribute/release_process.rst +++ b/docs/contribute/release_process.rst @@ -17,8 +17,12 @@ .. _release_process: -Apache TVM Release Process -========================== +Release Process +=============== + +.. contents:: + :depth: 2 + :local: The release manager role in TVM means you are responsible for a few different things: