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