On Thu, Feb 11, 2021 at 5:54 PM Amar Tumballi <amar@xxxxxxxxx> wrote:
On Thu, 11 Feb, 2021, 9:19 pm Xavi Hernandez, <xhernandez@xxxxxxxxxx> wrote:On Wed, Feb 10, 2021 at 1:33 PM Amar Tumballi <amar@xxxxxxxxx> wrote: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.I agree that it could be hard to enforce some rules, but what I'm seeing lately is that the clang-format version from Fedora 33 doesn't format the code the same way as a previous version with the same configuration in some cases (this also seems to happen with much older versions). This causes failures in the clang check that need manual modifications to update the patches.Ok, let's get moving with actual work than syntaxes. Ok with skipping!
If we could run a specific version within a container...
Y.
-------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
