[PATCH] D40988: Clang-format: add finer-grained options for putting all arguments on one line

Manuel Klimek via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Mar 15 02:12:32 PDT 2019


klimek added a comment.

In D40988#1430502 <https://reviews.llvm.org/D40988#1430502>, @MyDeveloperDay wrote:

> So @russellmcc  you've been bumping along this road nicely for 6 months, doing what people say... pinging every week or so in order to get your code reviewed, and you are getting no response.
>
> Was there anything you think people were objecting too other than the normal "its a high bar to get in" and "its complex in here"?
>
> I think its fairer to the person submitting a revision if they don't want feature in, we should give feedback as to why, but are we to assume silence is acceptance? how long do we wait? who is the decision maker?
>
> I've personally never met an Engineer who didn't like having more knobs to fiddle with so I don't believe the rational that having more options is bad as long as they don't interfere with each other, for that there is the "Beyonce rule", if adding an option breaks someone else then "if they liked it they should have put a test on it!"
>
> As far as I can tell this LGTM (I'm not the code owner, just someone wanting to help)
>
> In the meantime I have uploaded this patch to my fork, where I'm maintaining a clang-format with revisions that seem ok, but get stalled in the review process.
>
> https://github.com/mydeveloperday/clang-experimental


I think in this case there were no objections to the knob, but djasper had concerns about the code.
The problem in this case is that there was a 3 month pause between the review comment and the response, and for those intricate changes (and yes, clang-format's design is problematic here in that it makes these changes really hard to get right) it take a long time to think oneself into the problem to fully understand where it could go wrong.

Regarding the abstract argument brought up about tests - this is not about whether specific things break, but whether the code gets harder to maintain over time, which in the end will lead to nobody being able to make changes, regardless whether there is a reviewer or not - I think it's important we don't underestimate that risk & cost.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D40988/new/

https://reviews.llvm.org/D40988





More information about the cfe-commits mailing list