RFC: Introduce linting

Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

RFC: Introduce linting

Julien R
Hello,

I would like to propose adding linting to the mapproxy codebase and enforce it on the CI. The benefits of linting are mostly that they make reviews easier because reviewers don't have to point stylistic issues.

Johannes Weskamm originally proposed to introduce linting in this github issue:

I personally have experience with using flake8 professionally for multiple years and it works great. I started experimenting with setting up flake8 on the CI here:

If this is accepted, the plan would be to:
- Introduce flake8 on CI, but disable all errors at first
- Enable errors one by one in separate commits

This is to avoid submitting a massive PR touching everything. See the attached 'mapproxy_flake8_stats.txt' file for statistics about the "errors" found by flake8.

Best regards,
Julien

--
Julien Rebetez
Lead Software & Machine Learning Engineer
Picterra SA
Rue de la Mouline 8, 1022 Chavannes, Switzerland

_______________________________________________
MapProxy-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/mapproxy-dev

mapproxy_flake8_stats.txt (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Introduce linting

Tom Kralidis
Big +1 here.  We use flake8 as part of Travis/GitHub Actions in
numerous projects
and have found it valuable in code style alignment.  We will need to
address E501
(max-line-length), however this is configurable (like many flake8 settings).

I also like your approach of tackling flake8 errors iteratively.
Perhaps we can update
[1] with a checklist of all the error types so if anyone wants to jump
they can see the
state of things at a given point in time.

Big +1 from me.

..Tom

[1] https://github.com/mapproxy/mapproxy/issues/478


On Wed, Jan 13, 2021 at 2:36 AM Julien R <[hidden email]> wrote:

>
> Hello,
>
> I would like to propose adding linting to the mapproxy codebase and enforce it on the CI. The benefits of linting are mostly that they make reviews easier because reviewers don't have to point stylistic issues.
>
> Johannes Weskamm originally proposed to introduce linting in this github issue:
> https://github.com/mapproxy/mapproxy/issues/478
>
> I personally have experience with using flake8 professionally for multiple years and it works great. I started experimenting with setting up flake8 on the CI here:
> https://github.com/mapproxy/mapproxy/pull/486
>
> If this is accepted, the plan would be to:
> - Introduce flake8 on CI, but disable all errors at first
> - Enable errors one by one in separate commits
>
> This is to avoid submitting a massive PR touching everything. See the attached 'mapproxy_flake8_stats.txt' file for statistics about the "errors" found by flake8.
>
> Best regards,
> Julien
>
> --
> Julien Rebetez
> Lead Software & Machine Learning Engineer
> Picterra SA
> Rue de la Mouline 8, 1022 Chavannes, Switzerland
> www.picterra.ch
> _______________________________________________
> MapProxy-dev mailing list
> [hidden email]
> https://lists.osgeo.org/mailman/listinfo/mapproxy-dev
_______________________________________________
MapProxy-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/mapproxy-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Introduce linting

Johannes Weskamm
In reply to this post by Julien R

+1


Greetings,

Johannes

Am 13.01.21 um 08:36 schrieb Julien R:
Hello,

I would like to propose adding linting to the mapproxy codebase and enforce it on the CI. The benefits of linting are mostly that they make reviews easier because reviewers don't have to point stylistic issues.

Johannes Weskamm originally proposed to introduce linting in this github issue:

I personally have experience with using flake8 professionally for multiple years and it works great. I started experimenting with setting up flake8 on the CI here:

If this is accepted, the plan would be to:
- Introduce flake8 on CI, but disable all errors at first
- Enable errors one by one in separate commits

This is to avoid submitting a massive PR touching everything. See the attached 'mapproxy_flake8_stats.txt' file for statistics about the "errors" found by flake8.

Best regards,
Julien

--
Julien Rebetez
Lead Software & Machine Learning Engineer
Picterra SA
Rue de la Mouline 8, 1022 Chavannes, Switzerland

_______________________________________________
MapProxy-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/mapproxy-dev
-- 
  Dipl.-Geogr. Johannes Weskamm
  — Anwendungsentwickler —

  terrestris GmbH & Co. KG
  Kölnstraße 99
  53111 Bonn

  Tel: +49 (0)228 / 96 28 99 -555
  Fax: +49 (0)228 / 96 28 99 -57

  Email: [hidden email]
  Web: https://www.terrestris.de

  Amtsgericht Bonn, HRA 6835
  Komplementärin: terrestris Verwaltungsgesellschaft mbH
  vertreten durch: Torsten Brassat, Marc Jansen
  
  Informationen über Ihre gespeicherten Daten finden Sie auf
  unserer Homepage unter folgendem Link: 
  https://www.terrestris.de/datenschutzerklaerung/ 

_______________________________________________
MapProxy-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/mapproxy-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Introduce linting

Oliver Tonnhofer-5
In reply to this post by Julien R
I'm 0 on this.

The benefits are there, but the IMO huge downside is that it "destroys" the history of the code (git blame, etc.).
It would see this different if there is ongoing active development, but most parts are quite stable, were not changed in recent years and will likely not be edited in the near future. 

All the best,
Oliver

On 13. Jan 2021, at 08:36, Julien R <[hidden email]> wrote:

Hello,

I would like to propose adding linting to the mapproxy codebase and enforce it on the CI. The benefits of linting are mostly that they make reviews easier because reviewers don't have to point stylistic issues.

Johannes Weskamm originally proposed to introduce linting in this github issue:

I personally have experience with using flake8 professionally for multiple years and it works great. I started experimenting with setting up flake8 on the CI here:

If this is accepted, the plan would be to:
- Introduce flake8 on CI, but disable all errors at first
- Enable errors one by one in separate commits

This is to avoid submitting a massive PR touching everything. See the attached 'mapproxy_flake8_stats.txt' file for statistics about the "errors" found by flake8.

Best regards,
Julien

--
Julien Rebetez
Lead Software & Machine Learning Engineer
Picterra SA
Rue de la Mouline 8, 1022 Chavannes, Switzerland
<mapproxy_flake8_stats.txt>_______________________________________________
MapProxy-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/mapproxy-dev


_______________________________________________
MapProxy-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/mapproxy-dev