[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 Shyamsundar changed: What|Removed |Added Fixed In Version|glusterfs-5.0 |glusterfs-6.0 --- Comment #45 from

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 Shyamsundar changed: What|Removed |Added Status|POST|CLOSED Fixed In Version|

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 --- Comment #43 from Worker Ant --- REVIEW: https://review.gluster.org/21193 (xlators/experimental: move template files to '.c.in' type) posted (#5) for review on master by Xavi Hernandez -- You are receiving this mail because: You are on

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 --- Comment #42 from Worker Ant --- REVIEW: https://review.gluster.org/21351 (rfc.sh: fix the error in case clang-format is not present) posted (#1) for review on master by Amar Tumballi -- You are receiving this mail because: You are on

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 Nithya Balachandran changed: What|Removed |Added CC||nbala...@redhat.com ---

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 Worker Ant changed: What|Removed |Added Status|ASSIGNED|POST --- Comment #40 from Worker Ant

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 Amar Tumballi changed: What|Removed |Added Status|POST|ASSIGNED --- Comment #39 from Amar

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 --- Comment #38 from Worker Ant --- COMMIT: https://review.gluster.org/21164 committed in master by "Amar Tumballi" with a commit message- template files: revert clang Change-Id: If3925191d23afe83cbbdbc3cf0554c0a9c76d043 updates: bz#1564149

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 --- Comment #37 from Worker Ant --- REVIEW: https://review.gluster.org/21164 (template files: revert clang) posted (#1) for review on master by Amar Tumballi -- You are receiving this mail because: You are on the CC list for the bug.

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 --- Comment #36 from Worker Ant --- COMMIT: https://review.gluster.org/20892 committed in master by "Nigel Babu" with a commit message- clang-format: add the config file Also update the required documents and scripts to enable clang-format

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 Worker Ant changed: What|Removed |Added Status|NEW |POST --- Comment #35 from Worker Ant

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 --- Comment #34 from Nigel Babu --- Jenkins job is ready: https://review.gluster.org/#/c/20418/ Cannot merge this until we have a config file as it's a voting job. Yaniv, just to be clear, these machines are still VMs :) We only have 4

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 --- Comment #33 from Yaniv Kaul --- (In reply to Nigel Babu from comment #32) > We were blocked on Fedora builders. We procured them 2 weeks ago and they've > had some time to settle in. Infra-wise, we're nearly ready to go. I have a > job

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 Nigel Babu changed: What|Removed |Added Flags|needinfo?(nig...@redhat.com | |)

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 Yaniv Kaul changed: What|Removed |Added Flags||needinfo?(nig...@redhat.com

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 --- Comment #30 from Xavi Hernandez --- (In reply to Jeff Darcy from comment #24) > Personally I'm willing to go with the flow on those. I've laid out my > reasoning, other people seem less inclined to compromise, I think

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 --- Comment #29 from Nigel Babu --- Um, so I've been accepting the pull requests that come in more or less mostly so we can see what the output looks like. I'm going to wait for 24h for each of the remaining pull requests

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 --- Comment #28 from Xavi Hernandez --- (In reply to Aravinda VK from comment #27) > I prefer Right(`AlignEscapedNewlines=Right`), since Patch will have minimum > number of lines change on modification. For example, > >

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 --- Comment #27 from Aravinda VK --- (In reply to Xavi Hernandez from comment #19) > I made a mistake on one of the proposed values: > > AlignEscapedNewLinesLeft=false > > What I wanted to propose was to set it to

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 --- Comment #25 from Xavi Hernandez --- I don't want to start any battle. It was requested to create pull requests with the proposed changes and that's what I've done. I've not hidden that some of the changes are

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 --- Comment #23 from Xavi Hernandez --- The change will also include some of the controversial options: IndentCaseLabels=true SpaceBeforeParens=ControlStatements What should we do with them ? -- You are receiving this

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 --- Comment #22 from Xavi Hernandez --- Some issues I've seen: * Some initializations are formatted like this: char gfid_local[GF_UUID_BUF_SIZE] = { 0, }; I've tried to change

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 --- Comment #21 from Jeff Darcy --- The feedback so far has been strongly positive, but one issue has been raised. As Xavi mentioned, the PointerAlignment doesn't seem to work properly on lines affected by AlignConsecutive*.

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 --- Comment #19 from Xavi Hernandez --- I made a mistake on one of the proposed values: AlignEscapedNewLinesLeft=false What I wanted to propose was to set it to 'true' (which matches the explanation I gave). -- You

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 --- Comment #18 from Jeff Darcy --- Monday morning EST would be fine. -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 --- Comment #17 from Amar Tumballi --- Thanks for these pointers Jeff, makes the case even more critical to work towards. Shyam kindly agreed to make this a blocker for 4.1 branching out, which means, we have to come to

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 --- Comment #16 from Jeff Darcy --- FWIW, since this came up in the maintainers' meeting... The inconsistency of style has had a measurable negative impact on ramp-up time for new developers at Facebook. I'm sure the same

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 --- Comment #15 from Xavi Hernandez --- (In reply to Jeff Darcy from comment #13) > IndentCaseLabels: personally I don't like indenting case labels because they > create a gap with no close brace - unless people put

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 Amar Tumballi changed: What|Removed |Added Priority|unspecified |high ---

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 --- Comment #13 from Jeff Darcy --- IndentCaseLabels: personally I don't like indenting case labels because they create a gap with no close brace - unless people put braces around cases, which is a bad habit for other reasons

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 --- Comment #12 from Xavi Hernandez --- Using Chromium as base, basically the changes you propose seem ok. I would like to propose these additional changes: * I prefer to use 4 spaces for indent. 2 seems too few to me.

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 --- Comment #11 from Jeff Darcy --- Whatever we pick is going to require some tweaking. Some of the things I notice: * All use two-space indents, which I personally find atrocious. * All put a function's type on the same

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 --- Comment #10 from Amar Tumballi --- (Update from Nigel) Which format should we start with as base? Google/LLVM/Mozilla/Webkit/Chromium *

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 --- Comment #9 from Amar Tumballi --- (In reply to Nigel Babu from comment #7) > Alright. So we have agreement that this is a good idea. How do we want to > implement this? As a check or as a pre-commit hook? This is a

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 --- Comment #8 from Amar Tumballi --- (In reply to Shreyas Siravara from comment #6) > Whoever came up with this idea is a genius, and I totally ack it :) All credits for this to Shreyas Siravara! As the original idea

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 Nigel Babu changed: What|Removed |Added CC||nig...@redhat.com

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 Shreyas Siravara changed: What|Removed |Added CC||sshre...@fb.com

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 --- Comment #5 from Jeff Darcy --- I think we're all in agreement on spaces vs. tabs. Tabs are eight spaces wide, full stop, end of story. It's kind of survivable with eight-space indents, but any other indent width becomes a

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 --- Comment #4 from Shyamsundar --- Overall ack to the proposal. Nice call-out by Jeff on any other change requiring a big-bang addressing to be a part of the same. I can live with 4 spaces :), but no tabs (way too much

[Gluster-infra] [Bug 1564149] Agree upon a coding standard, and automate check for this in smoke

https://bugzilla.redhat.com/show_bug.cgi?id=1564149 Xavi Hernandez changed: What|Removed |Added CC|