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