sunithabeeram commented on a change in pull request #3925: readthedocs: Add contribution guidelines URL: https://github.com/apache/incubator-pinot/pull/3925#discussion_r263534211
########## File path: docs/contribution_guidelines.rst ########## @@ -54,13 +126,79 @@ All source code files should have license headers. To automatically add the head .. note:: -If you checkin third-party code or files, do not add license headers for them. Follow these instructions to ensure we -are compliant with Apache Licensing process. <TBD> +If you checkin third-party code or files, ensure they are distributed under Apache, MIT or BSD-3 licenses. +Do not add license headers for them. Follow these instructions to ensure we are compliant with Apache Licensing process: + +* Under ``incubator-pinot/licenses`` add a LICENSE-<newlib> file that has the license terms of the included library. +* Update the ``incubator-pinot/LICENSE`` file to indicate the newly added library file paths under the corresponding supported Licenses. +* Update the exclusion rules for ``license`` and ``rat`` maven plugins in the parent pom: ``incubator-pinot/pom.xml``. + +If attention to the licensing terms in not paid early on, they will be caught much later in the process, when we prepare to make a new release. +Updating code at that time to work with the right libraries at that time might require bigger refactoring changes and delay the release process. + +Creating a PR +^^^^^^^^^^^^^ + +* Verifying code-style + +Run the following command to verify the code-style before posting a PR + +.. code-block:: none + + mvn checkstyle:check * Run tests +Before you create a review request for the changes, make sure you have run the corresponding unit tests for your changes. +You can run individual tests via the IDE or via maven command-line. Finally run all tests locally by running ``mvn clean install -Pbin-dist``. + +For changes that are related to performance issues or race conditions, it is hard to write reliable tests, so we recommend running manual stress tests to validate the changes. You ``MUST`` note the manual tests done in the PR description. + * Push changes and create a PR for review +Commit your changes with a meaningful commit message. + +.. code-block:: none + + $ git add <files required for the change> + $ git commit -m "Meaningful oneliner for the change" + $ git push origin <your issue branch> + +After this, create a PullRequest in `github <https://github.com/apache/incubator-pinot/pulls>`_. Include the following information in the description: + +* The changes that are included in the PR. + +* Information on any implementation choices that were made. + +* Evidence of sufficient testing. You ``MUST`` indicate the tests done, either manually or automated. + +Once the PR is created, the code base is compiled and all tests are run via ``travis``. Make sure you followup on any issues flagged by travis and address them. +If you see test failures that are intermittent, ``please`` create an issue to track them. + +Once the ``travis`` run is clear, request reviews from atleast 2 committers on the project and be sure to gently to followup on the issue with the reviewers. + * Addressing comments and Rebasing +Once you receive comments on github on your changes, be sure to respond to them on github and address the concerns. +If any discussions happen offline for the changes in question, make sure to capture the outcome of the discussion, so others can follow along as well. + +It is possible that while your change is being reviewed, other changes were made to the master branch. Be sure to pull rebase your change on the new changes thus: + +.. code-block:: none + + # commit your changes + $git add <updated files> Review comment: Fixed ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
