Re: Automatic clang-format for GitHub PRs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On Wed, Feb 10, 2021 at 3:29 PM Xavi Hernandez <xhernandez@xxxxxxxxxx> wrote:
Hi all,

I'm wondering if enforcing clang-format for all patches is a good idea...

I've recently seen patches where clang-format is doing changes on parts of the code that have not been touched by the patch. Given that all files were already formatted by clang-format long ago, this shouldn't happen.

This means that as the clang-format version evolves, the formatting with the same configuration is not the same. This introduces unnecessary noise to the file history that I think it should be avoided.

Additionally, I've also seen some cases where some constructs are reformatted in an uglier or less clear way. I think it's very hard to come up with a set of rules that formats everything in the best possible way.

For all these reasons, I would say we shouldn't enforce clang-format to accept a PR. I think it's a good test to run to catch some clear formatting issues, but it shouldn't vote for patch acceptance.

What do you think ?


One thing I have noticed is, as long as some test is 'skipped', no one bothers to check. It would be great if the whole diff (in case of failure) is posted as a comment, so we can consider that while merging. I would request one to invest time on posting the failure message as a comment back into issue from jenkins if possible, and later implement skip behavior. Otherwise, considering we have >10 people having ability to merge patches, many people may miss having a look on clang-format issues.

-Amar
 
Regards,

Xavi
-------

Community Meeting Calendar:
Schedule -
Every 2nd and 4th Tuesday at 14:30 IST / 09:00 UTC
Bridge: https://meet.google.com/cpu-eiue-hvk

Gluster-devel mailing list
Gluster-devel@xxxxxxxxxxx
https://lists.gluster.org/mailman/listinfo/gluster-devel



--
--
https://kadalu.io
Container Storage made easy!

-------

Community Meeting Calendar:
Schedule -
Every 2nd and 4th Tuesday at 14:30 IST / 09:00 UTC
Bridge: https://meet.google.com/cpu-eiue-hvk

Gluster-devel mailing list
Gluster-devel@xxxxxxxxxxx
https://lists.gluster.org/mailman/listinfo/gluster-devel


[Index of Archives]     [Gluster Users]     [Ceph Users]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux