Merge pull request #174 from cta-observatory/docs_before_pull_request

Add in the documentation practices before opening a PR or before committing changes to an already open PR

Makefile

@@ -47,8 +47,8 @@ lint:
 	@flake8 magicctapipe
 
 env:
-	conda env create -n cta-dev -f environment.yml
-	source activate cta-dev
+	conda env create -n magic-lst -f environment.yml
+	source activate magic-lst
 
 develop:
 	pip install -e .

docs/developer-guide/getting-started.rst

@@ -209,7 +209,7 @@ we recommend starting a new branch like this:
 Edit the code
 ^^^^^^^^^^^^^
 
-and make as many commits as you want (more than one is generally
+Make as many commits as you want (more than one is generally
 better for large changes!).
 
 .. code-block:: sh
@@ -240,7 +240,7 @@ sub-module), check the style, and make sure the docs render correctly
    Therefore it's best to group related changes with ``git
    add <files>``. You may even commit only *parts* of a changed file
    using and ``git add -p``.  If you want to keep your git commit
-   history clean, learn to use commands like ``git commit --ammend``
+   history clean, learn to use commands like ``git commit --amend``
    (append to previous commit without creating a new one, e.g. when
    you find a typo or something small).
 
@@ -330,10 +330,39 @@ For differences between rebasing and merging and when to use which, see `this tu
 Create a *Pull Request*
 ^^^^^^^^^^^^^^^^^^^^^^^
 
-When you're happy, you create PR on on your github fork page by clicking
-"pull request".  You can also do this via *GitHub Desktop* if you have
-that installed, by pushing the pull-request button in the
-upper-right-hand corner.
+.. warning::
+   Before creating a pull request, please check the following:
+
+   * the code style is ok i.e. run:
+
+      .. code-block:: console
+
+         pre-commit run --all-files
+
+     and possibly fix files that needs changes
+
+   * the documentation builds without issues i.e. run:
+
+      .. code-block:: console
+
+         make doc
+
+     and have a look at the local documentation with your browser by opening ``docs/_build/html/index.html``
+
+   * the tests are passing i.e. run:
+
+      .. code-block:: console
+
+         make test
+
+     and fix the code if there are failing tests.
+
+When you are happy, you can create a pull request (PR):
+
+* if you work on a fork, on your github fork page by clicking "pull request". You can also do this via *GitHub Desktop* if you have
+  that installed, by pushing the pull-request button in the upper-right-hand corner.
+* if you work on the main repository, on the `magic-cta-pipe pull requests page <https://github.com/cta-observatory/magic-cta-pipe/pulls>`_,
+  by clicking on the `New pull request` button.
 
 Make sure to describe all the changes and give examples and use cases!
 
@@ -345,7 +374,11 @@ Wait for a code review
 Keep in mind the following:
 
 * At least one reviewer must look at your code and accept your
-  request. They may ask for changes before accepting.
+  request. They may ask for changes before accepting. Like before opening a PR,
+  please verify that code style is ok, that documentation builds fine and that
+  unit tests are not failing before committing new changes to the PR branch. This
+  avoids that the Continuos Integration runs for changes that you know already
+  are not passing all checks. See also :ref:`pull-requests` section for more info.
 * All unit tests must pass.  They are automatically run by *Travis* when
   you submit or update your pull request and you can monitor the
   results on the pull-request page.  If there is a test that you added
@@ -355,7 +388,8 @@ Keep in mind the following:
   feature is complete.
 * All documentation must build without errors. Again, this is checked
   by *Travis*.  It is your responsibility to run ``make doc`` and check
-  that you don't have any syntax errors in your docstrings.
+  that you don't have any syntax errors in your docstrings (check the
+  local documentation in your browser by opening the file ``docs/_build/html/index.html``)
 * All code you have written should follow the style guide (e.g. no
   warnings when you run the ``flake8`` syntax checker)
 

docs/developer-guide/pull-requests.rst

@@ -62,10 +62,17 @@ Keep in mind
 ------------
 
 * make sure you remember to update the **documentation** as well as the code!
-  (see the ``docs/`` directory), and make sure it builds with no errors
-  (``make doc``)
+  (see the ``docs/`` directory), and make sure it builds with no errors i.e. run
+  ``make doc`` and check the local documentation in your browser by opening the
+  file ``docs/_build/html/index.html``
 
-* pull requests that cause tests to fail on *Travis* will not be accepted until those tests pass
+* pull requests that cause tests to fail on *Travis* will not be accepted until those tests pass.
+  For this, check that the tests are passing on your local repository by running ``make test``
+  before pushing new changes to the PR
+
+* the code style will be checked. If you have ``pre-commit`` installed, this will be checked
+  automatically when you try to add a commit, possibly stopping the commit if the checks
+  are not passing
 
 .. * make sure to add a news fragment for the changelog.  In order to do this add a file to the directory ``docs/changes`` and use the following naming scheme
   ``<PULL REQUEST>.<TYPE>.rst`` (take a look at the ``README`` inside of the directory for more details). The file should contain a brief summary of the purpose of this pull request.