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 all 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
2 changes: 1 addition & 1 deletion .env-dev.dist
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
ENV=dev
SERVER_NAME=geotrek.local
SERVER_NAME=geotrek.localhost
POSTGRES_HOST=postgres
POSTGRES_PORT=5432
POSTGRES_USER=geotrek
Expand Down
25 changes: 25 additions & 0 deletions .githooks/pre-commit
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
#!/bin/bash

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

echo "Check dependency graph and fix requirements"

dockercompose run -T --rm web bash -c "pip-compile -q && pip-compile requirements-dev.in"



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
2 changes: 1 addition & 1 deletion .github/workflows/doc.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,4 @@ jobs:
run: |
cd docs/
pip install -r ./requirements.txt
make html
make html SPHINXOPTS="-W"
41 changes: 31 additions & 10 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,26 +7,47 @@ env:
DEBIAN_FRONTEND: noninteractive

jobs:
flake8:
name: Check code styling
runs-on: ubuntu-18.04
quality:
name: Checking dependency graph and code quality
runs-on: ubuntu-20.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
- name: Check dependency graph
run: |
flake8 geotrek
pip-compile -q
pip-compile -q requirements-dev.in

- name: Not evaluated values in migration files
- name: Verify dependency graph is ok
uses: tj-actions/verify-changed-files@v13
id: verify-changed-files
with:
files: |
requirements.txt
requirements-dev.txt

- name: Validating graph
if: steps.verify-changed-files.outputs.files_changed == 'true'
run: |
test $(ls geotrek/*/migrations/*.py | xargs grep -l srid | xargs grep -L SRID | wc -l) -eq 0
echo "Dependency file(s) changed: ${{ steps.verify-changed-files.outputs.changed_files }}"
core.setFailed('Please add your new dependencies in setup.py and/or requirements-dev.in then run pip-compile to add them in requirements. (see docs/contribute/development)')

- name: Flake8
run: |
flake8 geotrek
2 changes: 2 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,9 @@ jobs:
run: |
python3 -m pip install --upgrade pip setuptools wheel
pip3 wheel --wheel-dir=~/.wheel_dir -r requirements.txt
pip3 wheel --wheel-dir=~/.wheel_dir -r requirements-dev.txt
pip3 install --find-links=~/.wheel_dir -r requirements.txt
pip3 install --find-links=~/.wheel_dir -r requirements-dev.txt

- name: Create test required directories
run: |
Expand Down
19 changes: 11 additions & 8 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ ENV CUSTOM_SETTINGS_FILE="/opt/geotrek-admin/var/conf/custom.py"

WORKDIR /opt/geotrek-admin
RUN mkdir -p /opt/geotrek-admin/var/log /opt/geotrek-admin/var/cache

RUN adduser geotrek --disabled-password && chown geotrek:geotrek -R /opt
# Install postgis because raster2pgsl is required by manage.py loaddem
RUN apt-get update -qq && apt-get install -y -qq \
unzip \
Expand All @@ -42,16 +42,19 @@ 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
USER geotrek
RUN python3 -m venv /opt/venv
RUN /opt/venv/bin/pip install -U pip setuptools wheel
RUN /opt/venv/bin/pip install --no-cache-dir -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
COPY VERSION VERSION
COPY setup.cfg setup.cfg
COPY docker/* /usr/local/bin/
COPY --chown=geotrek:geotrek geotrek/ geotrek/
COPY --chown=geotrek:geotrek manage.py manage.py
COPY --chown=geotrek:geotrek VERSION VERSION
COPY --chown=geotrek:geotrek setup.cfg setup.cfg
COPY --chown=geotrek:geotrek docker/* /usr/local/bin/

ENTRYPOINT ["/bin/sh", "-e", "/usr/local/bin/entrypoint.sh"]
EXPOSE 8000
Expand Down
6 changes: 6 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ build-no-cache:
serve:
docker-compose up

deps:
docker-compose run --rm web bash -c "pip-compile -q && pip-compile -q requirements-dev.in"

flake8:
docker-compose run --rm web flake8 geotrek

messages:
docker-compose run --rm web ./manage.py makemessages -a --no-location

Expand Down
51 changes: 51 additions & 0 deletions docs/CONTRIBUTING.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
============
Contributing
============

Conventions
-----------

* 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 ?)
* Use flake8
* KISS & DRY as much as possible
* Elegant and generic is good, simple is better
* 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.


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

* ``docs/changelog.rst`` is up-to-date
* 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 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:

::

find geotrek | xargs egrep -n -i '(TODO|XXX|temporary|FIXME)'


Release
-------

On master branch:

* If need be, merge ``translations`` branch managed with https://weblate.makina-corpus.net
* Update files *VERSION*, *docs/conf.py* and *docs/changelog.rst* to remove ``+dev`` suffix and increment version (please use semver rules)
* Run ``dch -r -D RELEASED``, update version in the same way and save
* Commit with message 'Release x.y.z' to merge in ``master`` branch before release
* Add git tag X.Y.Z
* Update files *VERSION*, *docs/conf.py* and *docs/changelog.rst* to add ``+dev`` suffix
* Run ``dch -v <version>+dev --no-force-save-on-release`` and save
* Commit with message 'Back to development'
* Push branch and tag
* Add release on Github (copy-paste ``doc/changelog.rst`` paragraph)
* 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)
15 changes: 12 additions & 3 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,15 @@ CHANGELOG
- Fix intervention datatable list if one intervention has no target


**Development**

- New contributing guide (docs/CONTRIBUTING.rst).
- Development dependencies are now split in dedicated file.
- pip-tools and flake8 are now available in developer environment.
- Dependency graph is now checked in CI (see docs/contribute/development to how add a new dependency).
- New git pre-commit hook to check all is alright before commit (see docs/contribute/development).


2.95.0 (2023-01-24)
-----------------------

Expand Down Expand Up @@ -380,7 +389,7 @@ In preparation for HD Views developments (PR #3298)

- Upgrade mapentity to 8.2.1

**/!\ Regression /!\**
**! Regression !**

- System permissions on files output by `sync_rando` and `sync_mobile` commands were inadvertently changed to more restricted
with no reading allowed by group or other. This may cause trouble if your deployment relies on those permissions.
Expand Down Expand Up @@ -4169,8 +4178,7 @@ In order to enable those features under construction, add ``experimental = True`
:notes:

Give related permissions to the managers group in order to allow edition
(``add_flatpage``, ``change_flatpage``, ``delete_flatpage``,
``add_touristiccontent`` ...).
(``add_flatpage``, ``change_flatpage``, ``delete_flatpage``, ``add_touristiccontent`` ...).


0.27.2 (2010-10-14)
Expand Down Expand Up @@ -4354,6 +4362,7 @@ Since the map export have changed, empty the cache :
* Rework display of lists in detail pages, better factorization
* Removed links in logbook list for certain models
* Display messages in login page too (useful for redirections)

Support edition of several fields on the same map, via django-leaflet new feature (fixes #53)


Expand Down
3 changes: 2 additions & 1 deletion docs/conf.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import sphinx_rtd_theme # noqa
import datetime
# Geotrek documentation build configuration file, created by
# sphinx-quickstart on Wed May 15 09:50:19 2013.
#
Expand Down Expand Up @@ -41,7 +42,7 @@

# General information about the project.
project = 'Geotrek'
copyright = '2013-2021, Makina Corpus'
copyright = f'2013-{datetime.date.today().year}, Makina Corpus'

# The version info for the project you're documenting, acts as replacement for
# |version| and |release|, also used in various other places throughout the
Expand Down
Loading