[Gluster-devel] Automatic clang-format for GitHub PRs

Amar Tumballi amar at kadalu.io
Wed Feb 10 12:33:19 UTC 2021

On Wed, Feb 10, 2021 at 3:29 PM Xavi Hernandez <xhernandez at redhat.com>

> 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.


> 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 at gluster.org
> https://lists.gluster.org/mailman/listinfo/gluster-devel

Container Storage made easy!
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.gluster.org/pipermail/gluster-devel/attachments/20210210/7df56912/attachment.html>

More information about the Gluster-devel mailing list