[postgis-devel] Code formatting digging in

Raúl Marín Rodríguez rmrodriguez at carto.com
Thu Feb 22 12:54:21 PST 2018


> As for clang-format:
>   Ubuntu 17.10 has versions 3.8, 3.9, 4.0 and 5.0
>   Debian  8.8  has versions 3.4, 3.5, 3.8
> The fact they ship multiple versions makes me think they
> also behave differently.

Clang-format is tied to Clang (same release), so a change in version
doesn't necessarily mean a change in behaviour, but it might.

> Only formatting the changed lines is an interesting idea,
> but then how can we ensure style is retained ?
>
> If we ever enforce a style we'll want CI to check style is the
> official style, and not allow anything else, so we want a full
> reformat anyway at that point.

Not necessarily. You can setup clang-format to only analyze the patch /
commit
and format that. This way we can minimize the impact of using different
clang
releases producing slightly different styles. I can live with
someone a month from now changing the placement of a parentheses because
she uses clang 3.5 instead of 5. That is, as long as it's a parentheses
in the code she's modifying and not all parentheses in the project.

> I don't see the benefit of this. I use GCC to compile, btw, so I'd
> have to install clang-format separately.

The same happens with any tool, but I think that what Komzpa was implying is
that you could expect a formatter using the compiler parser to be more
robust
than a standalone app.

> It's not installed, because it's meant as tool for developers, not for
> regular users. That is, it's in src/tools/pgindent and that's it.

I compile and package my own Postgresql (without pgident) but I wouldn't
expect all PostGIS developers or possible contributors to do the same.

> Just to give more info: the version of astyle that QGIS ships
> is 3.1, they just embed the full source code in qgis codebase.

I like the idea of having the style tool integrated in the project but I
value
more making it as automatic as posible, which could be a check as part of
the
CI.

The issue I see with a full check is that if a commiter doesn't follow it
then all PRs will be broken because of this, similar to what happens now
with
the tests, and I would rather have them analyzing just the patch.

Anyway, I could live with almost anything if it's integrated in the CI.

Regards
-- 

*Raúl Marín Rodríguez *carto.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/postgis-devel/attachments/20180222/15f588f1/attachment.html>


More information about the postgis-devel mailing list