Some points on clang-format usage

Stephan Bergmann sbergman at redhat.com
Mon Mar 12 10:40:49 UTC 2018


On 11/03/18 07:44, Chris Sherlock wrote:
> And a quick comment - back in the day I split OutputDevice into seperate files. People were actually very supportive of this change. You asked me why I split out the files, but bitmap functions are already split. So how is it useful? Well, we could combine all the functions into one file, because bitmap3.cxx as a filename is less than useful.

I asked for a rationale, yes.  To me at least, that change 
(<https://gerrit.libreoffice.org/#/c/50530/>) looked rather random.  And 
combined with the fact that it cosmetically changed the moved code, that 
made me a bit nervous (because that makes tracking the code's history 
unnecessarily harder).

> I have been consulting with Tomasz about my code changes, by the way. He has been working on the bitmap code, and I’ve been writing bitmap tests, which apparently are useful. If you don’t like the unit tests, please let me know - after all, I’ve stoped looking at the SAL as it got people’s noses out of joint. Given I’ve previously worked on the VCL and even fixed bugs, I thought that would be alright.

Why do you think that I might not like certain unit tests?

> Stephan, are you actively working on vcl bitmap code? I am happy to add you as a reviewer to my gerrit commits, would you like me to do this in future? Are there any other modules you want me to add you as a reviewer?

No, I'm not actively working on vcl bitmap code, and I have little 
insight into that code anyway.  However, what I do care about is that if 
I have to look into that code, any code, that it isn't unnecessarily 
hard to understand what changes have been made to the code over time, 
for which reasons.

>> On 6 Mar 2018, at 8:25 pm, Stephan Bergmann <sbergman at redhat.com> wrote:
>>> On 17.11.2017 10:03, Stephan Bergmann wrote:
>>> * Don't reformat when moving an existing file
>>> When moving an old file that is listed in solenv/clang-format/blacklist (so doesn't automatically get reformatted upon commit), adapt solenv/clang-format/blacklist to contain the new name, instead of provoking a reformat upon commit.  Otherwise, if the reformatted code looks sufficiently different from the original, it's hard to impossible to keep track in the git history of which file was moved where.
>>
>> Please also follow this advice when moving part of an existing file out to a new file, as happened e.g. at <https://cgit.freedesktop.org/libreoffice/core/commit/?id=bdd62b4c327ab894dbba00fe2e07696c1b7d9de6> "vcl: split painting bitmap functions to bitmappaint.cxx".


More information about the LibreOffice mailing list