Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Docs before pull request #174

Merged
merged 2 commits into from
Nov 4, 2023
Merged

Docs before pull request #174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
294cbad
Update environment name.
aleberti Nov 4, 2023
e4a5b6b
Add checks to be done before opening PR (or before committing new cha…
aleberti Nov 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 .
Expand Down
50 changes: 42 additions & 8 deletions docs/developer-guide/getting-started.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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).

Expand Down Expand Up @@ -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!

Expand All @@ -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
Expand All @@ -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)

Expand Down
13 changes: 10 additions & 3 deletions docs/developer-guide/pull-requests.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down