[gdal-dev] Automating code style enforcement ?

[Blake Thompson]

All,

Just wanted to contribute some experiences we have gained at Mapbox while
doing our C++ work on attempting to work towards a common format on some
libraries. We have found that clang format is very helpful, but it is still
also very good if you have written documentation on how to follow the style
at some location. RFC seems to be the right track to gather thoughts.

The best way we have found so far to use clang format has been to check in
a ".clang-format" file with each repository. Once this is done, we put in a
script in our makefile to automate the updating process
https://github.com/mapbox/wagyu/blob/master/Makefile#L95-L96 as an example.

A couple thing we have realized:

- Even experienced with a style often make mistakes as we are often
shifting between projects (as they have different styles)
- A `make indent` (clang format) is a simple way to fix problems and check
them in on occasion (such as after big changes or before releases)
- Optimize the style for reading now for writing of the code
- If you want to be really strict you can have style checked as part of
testing (I am not personally a huge fan of this).

Thanks,

Blake Thompson


On Fri, May 5, 2017 at 1:04 PM, Kurt Schwehr <schwehr at gmail.com> wrote:

> astyle would certainly work too.
>
> I find clang-format more robust than almost all other formatting tools as
> Mateusz described.  As for workflow, there are multiple possible routes as
> to how to go about this.  I personally write pretty messy code when I'm
> working and then just do a format pass before submitting.  If the editor
> I'm using supports formatting, great.  However, the command line is super
> easy.  I also know a number of people who work on code only after applying
> their own person clang format style to code and then transform back before
> submitting.  And there is the critical ability to shut off the formatter
> for sections of code where the formatter will do something seriously
> wrong... the classic example is matrices in test files.
>
> Alternatively, we could easily have astyle as a first pass to add braces
> before clang-format if someone wanted to implement that.  But there
> clang-tidy as Mateusz said with:
>
> https://clang.llvm.org/extra/clang-tidy/checks/readability-
> braces-around-statements.html
>
> with lots of other check....
>
> https://clang.llvm.org/extra/clang-tidy/checks/list.html
>
> On Fri, May 5, 2017 at 4:46 AM, Mateusz Loskot <mateusz at loskot.net> wrote:
>
>> On 5 May 2017 at 13:35, Even Rouault <even.rouault at spatialys.com> wrote:
>> >
>> > In case we want to have a policy regarding braces around single
>> statements,
>> > one finding coming from the openjpeg discussion I mentionned :
>> >
>> > """
>> > Moreover, concerning braces around single statements, it seems astyle
>> does
>> > this with the add-braces option:
>> > http://astyle.sourceforge.net/astyle.html#_add-braces
>> > As far as I understood, clang-format does not, and requires another tool
>> > (clang-tidy) to deal with these cases. So one more point for astyle.
>> > """
>>
>> Rather, it is a separation of concerns: linting vs formatting
>> AFAIU, clang-format does not modify any code (ie. add tokens).
>>
>> I'm not clang-format advocate. I'll repeat myself:
>> My personal preference is to use clang-format.
>> I proposed it to GEOS via GEOS RFC.
>> Kurt found the RFC useful and is using it as a template
>> to prepare one for GDAL.
>> IMHO, at the end of the day, tool does not matter as long as proposed
>> goals are achieved.
>>
>> Best regards,
>> --
>> Mateusz Loskot, http://mateusz.loskot.net
>> _______________________________________________
>> gdal-dev mailing list
>> gdal-dev at lists.osgeo.org
>> https://lists.osgeo.org/mailman/listinfo/gdal-dev
>>
>
>
>
> --
> --
> http://schwehr.org
>
> _______________________________________________
> gdal-dev mailing list
> gdal-dev at lists.osgeo.org
> https://lists.osgeo.org/mailman/listinfo/gdal-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/gdal-dev/attachments/20170505/8bef5876/attachment.html>