<div dir="ltr"><div class="gmail_extra"><div><div>> As for clang-format:</div><div>> Ubuntu 17.10 has versions 3.8, 3.9, 4.0 and 5.0</div><div>> Debian 8.8 has versions 3.4, 3.5, 3.8</div><div>> The fact they ship multiple versions makes me think they</div><div>> also behave differently.</div><div><br></div><div>Clang-format is tied to Clang (same release), so a change in version</div><div>doesn't necessarily mean a change in behaviour, but it might.</div><div><br></div><div>> Only formatting the changed lines is an interesting idea,</div><div>> but then how can we ensure style is retained ?</div><div>></div><div>> If we ever enforce a style we'll want CI to check style is the</div><div>> official style, and not allow anything else, so we want a full</div><div>> reformat anyway at that point.</div><div><br></div><div>Not necessarily. You can setup clang-format to only analyze the patch / commit</div><div>and format that. This way we can minimize the impact of using different clang</div><div>releases producing slightly different styles. I can live with</div><div>someone a month from now changing the placement of a parentheses because</div><div>she uses clang 3.5 instead of 5. That is, as long as it's a parentheses</div><div>in the code she's modifying and not all parentheses in the project.</div><div><br></div><div>> I don't see the benefit of this. I use GCC to compile, btw, so I'd</div><div>> have to install clang-format separately.</div><div><br></div><div>The same happens with any tool, but I think that what Komzpa was implying is</div><div>that you could expect a formatter using the compiler parser to be more robust</div><div>than a standalone app.</div><div><br></div><div>> It's not installed, because it's meant as tool for developers, not for</div><div>> regular users. That is, it's in src/tools/pgindent and that's it.</div><div><br></div><div>I compile and package my own Postgresql (without pgident) but I wouldn't</div><div>expect all PostGIS developers or possible contributors to do the same.</div><div><br></div><div>> Just to give more info: the version of astyle that QGIS ships</div><div>> is 3.1, they just embed the full source code in qgis codebase.</div><div><br></div><div>I like the idea of having the style tool integrated in the project but I value</div><div>more making it as automatic as posible, which could be a check as part of the</div><div>CI.</div><div><br></div><div>The issue I see with a full check is that if a commiter doesn't follow it</div><div>then all PRs will be broken because of this, similar to what happens now with </div><div>the tests, and I would rather have them analyzing just the patch.</div><div><br></div><div>Anyway, I could live with almost anything if it's integrated in the CI.</div></div><div><br></div><div>Regards</div>-- <br><div class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><b style="font-size:12.8px"><font color="#666666">Raúl Marín Rodríguez <br></font></b><a href="http://carto.com/" style="font-size:12.8px" target="_blank">carto.com</a><div><br></div></div></div></div></div></div></div>
</div></div>