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.

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

Improve quality check and contributing guide #3430

Merged
merged 22 commits into from
Jan 27, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
19 changes: 19 additions & 0 deletions .githooks/pre-commit
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#!/bin/bash

function dockercompose {
# use docker-compose then fallback to new docker compose
docker-compose "$@"
docker compose "$@"
}

echo "run Flake8..."

dockercompose run -T --rm web flake8 geotrek
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about checking changed files only?

Copy link
Member Author

@submarcos submarcos Jan 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't check changed files, we check all files as actually

status=$?

if test $status -eq 1
then
exit $status
else
echo "Flake8 check is ok..."
fi
16 changes: 9 additions & 7 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,23 @@ jobs:
runs-on: ubuntu-18.04

steps:
- uses: actions/checkout@v2
- uses: actions/checkout@v3

- name: Not evaluated values in migration files
run: |
test $(ls geotrek/*/migrations/*.py | xargs grep -l srid | xargs grep -L SRID | wc -l) -eq 0


- name: Set up Python 3.8
uses: actions/setup-python@v4
with:
python-version: 3.8 # lint with minimal version supported (3.8 in 18.04)

- name: Install dependencies
run: |
pip3 install --upgrade pip
pip3 install flake8
pip3 install --upgrade pip wheel setuptools
pip3 install -r requirements-dev.txt -U

- name: Flake8
run: |
flake8 geotrek

- name: Not evaluated values in migration files
run: |
test $(ls geotrek/*/migrations/*.py | xargs grep -l srid | xargs grep -L SRID | wc -l) -eq 0
4 changes: 3 additions & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,12 @@ RUN apt-get update -qq && apt-get install -y -qq \
apt-get install -y --no-install-recommends postgis && \
apt-get clean all && rm -rf /var/lib/apt/lists/* && rm -rf /var/cache/apt/*

COPY requirements.txt requirements.txt
RUN python3 -m venv /opt/venv
RUN /opt/venv/bin/pip install -U pip setuptools wheel
COPY requirements.txt requirements.txt
RUN /opt/venv/bin/pip install --no-cache-dir -r requirements.txt -U
COPY requirements-dev.txt requirements-dev.txt
RUN /opt/venv/bin/pip install --no-cache-dir -r requirements-dev.txt -U

COPY geotrek/ geotrek/
COPY manage.py manage.py
Expand Down
39 changes: 29 additions & 10 deletions docs/contribute/development.rst
Original file line number Diff line number Diff line change
Expand Up @@ -36,20 +36,26 @@ Conventions
* Use flake8
* KISS & DRY as much as possible
* Elegant and generic is good, simple is better
* Before contributing, open an issue and discuss about it with community (is it a bug or a feature ? What is the best way to achieve my goal ?)
* Separate bug fixes and new features in several pull requests.
* Open a new Pull Request in "Draft" status until tests passed. Use at least 'bug', 'improvement' or 'feature' label.
* Commits messages are explicit and mention issue number (``(ref #12)`` or ``(fixes #23)``)
* Features are developed in a branch and merged from Github pull-requests. A git hook to is available to prevent pushing to master, to enable it, developpers should run the following command from root directory (`Geotrek-admin/`) : `ln -s -f ../../.githooks/pre-push .git/hooks/pre-push`
* Features are developed in a branch and merged from Github pull-requests.
* Several git hooks are available. Install them with following commands:
* pre-push: `ln -s -f ../../.githooks/pre-push .git/hooks/pre-push`
* pre-commit: `ln -s -f ../../.githooks/pre-commit .git/hooks/pre-commit`


Definition of done
------------------

* ``docs/changelog.rst`` is up-to-date
* A unit-test covers the bugfix or the new feature
* An explicit unit-test covers the bugfix or the new feature.
* A frontend test (:path:jstests/nav-\*.js) covers the navigation bug fix or feature
* A JS *Mocha* test (:path:jstests/tests.\*.js) covers the JavaScript bug fix or feature
* Unit-tests coverage is above or at least equal with previous commits
* Settings have default value in ``settings/base.py`` or ``conf/settings-default.ini``
* Installation instructions are up-to-date
* Unit-tests total coverage is above or at least equal with previous commits. Patch coverage is 100% on new lines.
* Settings have default value in ``settings/base.py``
* Installation instructions and documentation are up-to-date

Check TODO in the source tree:

Expand All @@ -76,16 +82,29 @@ On master branch:
* When creating a new release 'x.y.z' on github, Github actions will generate the .deb package file, and publish it on https://packages.geotrek.fr (see ``.circleci/config.yml`` file for details)


Developement
============
Development
===========

Adding or upgrade dependencies
------------------------------

Consider using pip-tools to manage dependencies.

* add your dependency in setup.py for general dependency, requirements-dev.in for dev dependency, then run :

::

docker-compose run --rm web pip-compile
docker-compose run --rm web pip-compile requirements-dev.in


Model modification
------------------

::

docker-compose run web ./manage.py makemigrations <appName>
docker-compose run web ./manage.py migrate
docker-compose run --rm web ./manage.py makemigrations <appName>
docker-compose run --rm web ./manage.py migrate

.. note ::

Expand Down Expand Up @@ -236,7 +255,7 @@ Data only:

::

docker-compose run web ./manage.py flush
docker-compose run --rm web ./manage.py flush

Restore existing Database
-------------------------
Expand Down
2 changes: 2 additions & 0 deletions requirements-dev.in
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
pip-tools
flake8
32 changes: 32 additions & 0 deletions requirements-dev.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
#
# This file is autogenerated by pip-compile with Python 3.8
# by the following command:
#
# pip-compile requirements-dev.in
#
build==0.10.0
# via pip-tools
click==8.1.3
# via pip-tools
flake8==6.0.0
# via -r requirements-dev.in
mccabe==0.7.0
# via flake8
packaging==23.0
# via build
pip-tools==6.12.1
# via -r requirements-dev.in
pycodestyle==2.10.0
# via flake8
pyflakes==3.0.1
# via flake8
pyproject-hooks==1.0.0
# via build
tomli==2.0.1
# via build
wheel==0.38.4
# via pip-tools

# The following packages are considered to be unsafe in a requirements file:
# pip
# setuptools