<div dir="auto">No objections from me. Anything we can do to drive consistency across the codebase is always welcome. Thanks for taking care of this! Now to adopt standard naming conventions for variables, functions, etc…</div><div dir="auto"><br></div><div dir="auto">+1</div><div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Sep 28, 2023 at 9:24 AM Even Rouault via MapServer-dev <<a href="mailto:mapserver-dev@lists.osgeo.org">mapserver-dev@lists.osgeo.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br>
<br>
Below text from <a href="https://github.com/MapServer/MapServer/pull/6937" rel="noreferrer" target="_blank">https://github.com/MapServer/MapServer/pull/6937</a>:<br>
<br>
I propose to apply to Mapserver the same set of tools that have been <br>
applied to GDAL and PROJ recently to have consistent code formatting for <br>
C/C++ and .py files<br>
<br>
This uses the pre-commit tool that installs hooks run by git commit to <br>
automatically apply the formatting rules:<br>
<br>
- clang-format for C/C++ code using a .clang-format style based on the <br>
LLVMStyle with a minimal customization to do 2-space indentation (which <br>
is the general practice in Mapserver), and not sorting includes (as this <br>
breaks compilation due to includes unfortunately not being <br>
clean/standalone, with inconsistencies like "a.h needs a type of b.h, <br>
but b.h includes a.h"). Third-party code is excluded from reformating <br>
(in .pre-commit-config.yaml)<br>
<br>
- black and isort to format Python code<br>
<br>
- flake8 to enforce some rules on Python code (with tons of exceptions <br>
in .flake8, directly borrowed from the ones of GDAL). I had to manually <br>
fixup a few things in the .py files<br>
<br>
Cf <a href="https://gdal.org/development/rfc/rfc69_cplusplus_formatting.html" rel="noreferrer" target="_blank">https://gdal.org/development/rfc/rfc69_cplusplus_formatting.html</a> on <br>
how to install pre-commit (one time operation to do), and <br>
<a href="https://gdal.org/development/dev_practices.html#commit-hooks" rel="noreferrer" target="_blank">https://gdal.org/development/dev_practices.html#commit-hooks</a><br>
<br>
A .git-blame-ignore-revs file is also added to identify the reformatting <br>
commit as to be ignored for the purpose of "git blame". It needs to be <br>
manually declared to git with "git config blame.ignoreRevsFile <br>
.git-blame-ignore-revs", for local use (one time operation) but it is <br>
automatically pickedup by GitHub.<br>
<br>
We should probably apply that to the stable branch as well to make <br>
backports easier.<br>
<br>
This also adds a .github/workflows/code_checks.yml GitHub Action that <br>
runs pre-commit on pull requests to detect violations. Such sitaution <br>
shouldn't happen for developers that have already set it up locally, but <br>
useful to catch issues coming from casual contributors (e.g. simulation <br>
of a commit that would break formatting: <br>
<a href="https://github.com/rouault/mapserver/actions/runs/6340096023/job/17220740975" rel="noreferrer" target="_blank">https://github.com/rouault/mapserver/actions/runs/6340096023/job/17220740975</a>)<br>
<br>
Upon acceptance, I'll add a "Code formatting rules" section to <br>
<a href="https://mapserver.org/development/index.html" rel="noreferrer" target="_blank">https://mapserver.org/development/index.html</a> to gather all above information<br>
<br>
Even<br>
<br>
-- <br>
<a href="http://www.spatialys.com" rel="noreferrer" target="_blank">http://www.spatialys.com</a><br>
My software is free, but my time generally not.<br>
<br>
_______________________________________________<br>
MapServer-dev mailing list<br>
<a href="mailto:MapServer-dev@lists.osgeo.org" target="_blank">MapServer-dev@lists.osgeo.org</a><br>
<a href="https://lists.osgeo.org/mailman/listinfo/mapserver-dev" rel="noreferrer" target="_blank">https://lists.osgeo.org/mailman/listinfo/mapserver-dev</a><br>
</blockquote></div></div>