On Thu, Feb 11, 2021 at 5:54 PM Amar Tumballi <a...@kadalu.io> wrote:
> > > On Thu, 11 Feb, 2021, 9:19 pm Xavi Hernandez, <xhernan...@redhat.com> > wrote: > >> On Wed, Feb 10, 2021 at 1:33 PM Amar Tumballi <a...@kadalu.io> wrote: >> >>> >>> >>> On Wed, Feb 10, 2021 at 3:29 PM Xavi Hernandez <xhernan...@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. >>> >> >> 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@gluster.org > 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@gluster.org https://lists.gluster.org/mailman/listinfo/gluster-devel
