Re: RFC: Running clang-format across all Plasma (and more?) repos

2019-07-14 Thread David Edmundson
>Aren't the problems described in task
https://phabricator.kde.org/T11214 specific enough?

There's one comment about constructor initialisers, which seems resolvable.

So if there's anything else, then yes, please be more specific.

If we find we we can't do this until we get some more features in
clang that's a perfectly valid resolution, but lets get a fully
comprehensive list of the changes we would need.

David


Re: RFC: Running clang-format across all Plasma (and more?) repos

2019-07-14 Thread Alexey Min
> There's one comment about constructor initialisers, which seems resolvable.

If that  solvable, then it's great. I'm in favour of this idea, maybe
even extend to frameworks repos?

> So if there's anything else, then yes, please be more specific.

No, so far that was the only problem we've found. If we find something
else, we'll come back with more specifics ;) Apparently it needs more
testing on larger code base.


Re: RFC: Running clang-format across all Plasma (and more?) repos

2019-07-14 Thread Albert Astals Cid
El dijous, 11 de juliol de 2019, a les 16:18:08 CEST, David Edmundson va 
escriure:
> One topic discussed at the recent Plasma sprint was that we should run
> a code formatting tool (clang-format) over all our repos to ease all
> future review comments about whitespace.
> 
> All new contributions simply have to run the same tool and we get
> consistent code without having to comment on every minor thing in a
> review individually.
> 
> I've written up a wall of text outlining steps, challenges etc.
> https://phabricator.kde.org/T11214
> 
> Does anyone have any thoughts / objections?

If we don't get some sort of precommit hook or "Merge Request" stage support 
it'll be somewhat useless.

"No one" will run the tool manually before doing a commit, this means that 
you'll need to run the tool every N months because commits go ignoring the 
format. I don't think that'd be sustainable in the long runo

I know "automation" is in your plan, but AFAICS only as optional, i don't 
really think that's optional.

Cheers,
  Albert

> 
> David
>