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

[Amar Tumballi]

On Wed, Feb 10, 2021 at 3:29 PM Xavi Hernandez <xhernandez at redhat.com>
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 at gluster.org
> https://lists.gluster.org/mailman/listinfo/gluster-devel
>
>

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