From a012b6ebdaa0a396b533dd997ace4776fc12bd65 Mon Sep 17 00:00:00 2001 From: yuvipanda Date: Tue, 31 Jul 2018 12:36:16 -0700 Subject: [PATCH 1/3] Add documentation on code review guidelines --- docs/contributing/code-review.rst | 44 +++++++++++++++++++++++++++++++ docs/contributing/docs.rst | 2 ++ docs/index.rst | 1 + 3 files changed, 47 insertions(+) create mode 100644 docs/contributing/code-review.rst diff --git a/docs/contributing/code-review.rst b/docs/contributing/code-review.rst new file mode 100644 index 0000000..7c56ca2 --- /dev/null +++ b/docs/contributing/code-review.rst @@ -0,0 +1,44 @@ +.. _contributing/code-review: + +====================== +Code Review guidelines +====================== + +This document outlines general guidelines to follow when you are making +or reviewing a Pull Request. + +Have empathy +============ + +We recommend reading `On Empathy & Pull Requests `_ +and `How about code reviews `_ +to learn more about being empathetic in code reviews. + +Write documentation +=================== + +If your pull request touches any code, you must write or update documentation +for it. For this project, documentation is a lot more important than the code. +If a feature is not documented, it does not exist. If a behavior is not documented, +it is a bug. + +Do not worry about having perfect documentation! Documentation improves over +time. The requirement is to have documentation before merging a pull request, +not to have *perfect* documentation before merging a pull request. + +See :ref:`contributing/docs` for guidelines on writing documentation. + +Write tests +=========== + +If your pull request touches any code, you must write unit or integration tests +to exercise it. This helps validate & communicate that your pull request works +the way you think it does. It also makes sure you do not accidentally break +other code, and makes it harder for future pull requests to break the code +added in your pull request. + +Since TLJH is a distribution that integrates many JupyterHub components, +integration tests provide more value for effort than unit tests do. Unit +tests are easier to write & faster to run, so if the code being changed +feels exhaustively unit-testable, write unit tests too. When in doubt, +add more tests. \ No newline at end of file diff --git a/docs/contributing/docs.rst b/docs/contributing/docs.rst index 9b12301..eb6dfbc 100644 --- a/docs/contributing/docs.rst +++ b/docs/contributing/docs.rst @@ -1,3 +1,5 @@ +.. _contributing/docs: + ===================== Writing documentation ===================== diff --git a/docs/index.rst b/docs/index.rst index 9f68ce7..a8c887d 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -116,4 +116,5 @@ to people contributing in various ways. :titlesonly: contributing/docs + contributing/code-review contributing/dev-setup From b5e6a27fa85b53c136379aba490e8fe2e6a72367 Mon Sep 17 00:00:00 2001 From: yuvipanda Date: Tue, 31 Jul 2018 12:55:01 -0700 Subject: [PATCH 2/3] Add a pull request template --- docs/PULL_REQUEST_TEMPLATE.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 docs/PULL_REQUEST_TEMPLATE.md diff --git a/docs/PULL_REQUEST_TEMPLATE.md b/docs/PULL_REQUEST_TEMPLATE.md new file mode 100644 index 0000000..1798476 --- /dev/null +++ b/docs/PULL_REQUEST_TEMPLATE.md @@ -0,0 +1,4 @@ + - [ ] Add / update documentation + - [ ] Add tests + + \ No newline at end of file From c6f2ecfd7fae85ca9fedcae9b450cd0735295c5f Mon Sep 17 00:00:00 2001 From: yuvipanda Date: Tue, 31 Jul 2018 12:59:24 -0700 Subject: [PATCH 3/3] Soften tone for code review guidelines --- docs/contributing/code-review.rst | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/contributing/code-review.rst b/docs/contributing/code-review.rst index 7c56ca2..1c2f406 100644 --- a/docs/contributing/code-review.rst +++ b/docs/contributing/code-review.rst @@ -24,7 +24,9 @@ it is a bug. Do not worry about having perfect documentation! Documentation improves over time. The requirement is to have documentation before merging a pull request, -not to have *perfect* documentation before merging a pull request. +not to have *perfect* documentation before merging a pull request. If you +are new and not sure how to add documentation, other contributors will +be happy to guide you. See :ref:`contributing/docs` for guidelines on writing documentation. @@ -41,4 +43,7 @@ Since TLJH is a distribution that integrates many JupyterHub components, integration tests provide more value for effort than unit tests do. Unit tests are easier to write & faster to run, so if the code being changed feels exhaustively unit-testable, write unit tests too. When in doubt, -add more tests. \ No newline at end of file +add more tests. + +If you are unsure what kind of tests to add for your pull request, other +contributors to the repo will be happy to help guide you! \ No newline at end of file