On Fri, Dec 14, 2018 at 1:58 PM Sylvestre Ledru <sle...@mozilla.com> wrote:
> We have more and more tools at review phase (clang-format, flake8, eslint, 
> clang-tidy, codespell, etc) which propose some auto-fixes.

Honestly I find it quite annoying when I'm trying to review a patch
and the phabricator diff is filled up with bot comments about
formatting. As a reviewer, I shouldn't have to think about formatting,
and it shouldn't fill up my screen and prevent me from looking at the
actual code.

> Currently, the turn around time of the tools is 14m on average which is 
> usually faster than the reviewer looking at the patch.

Usually, but not always. And 14 minutes is still long enough that the
person writing the patch has context-switched away and is in the
middle of doing something else when the bot comes a-knocking.

> If Phabricator provides the capability, we could have the bot automatically 
> proposing a new patchset on top on the normal one.
> The reviewer would look at the updated version.

This would be fine for the reviewer, but...

> The main drawback would be that developer would have to retrieve the updated 
> patch
> before updating it.

That's a chore too. For the developer writing the patch I feel like
there should be a way to just say "please format this patch in-place
for me" (basically what I expected `./mach clang-format` to do, except
it doesn't quite - see bug 1514279) and even that should be opt-in.
IMO the default behaviour should just be to automatically apply
formatting prior to submission, without anybody having to think about
it except in cases where the developer needs to apply "//clang-format
off" blocks.
dev-platform mailing list

Reply via email to