Re: Clang-Formatter for GlusterFS.

[Ravishankar N <ravishankar@xxxxxxxxxx>]

On 09/12/2018 07:31 PM, Amar Tumballi wrote:

Top posting:

All is well at the tip of glusterfs master branch now.

We will post a postmortem report of events and what went wrong in this activity, later.

With this, Shyam, you can go ahead with release-v5.0 branching.

-Amar

On Wed, Sep 12, 2018 at 6:21 PM, Amar Tumballi <atumball@xxxxxxxxxx> wrote:

On Wed, Sep 12, 2018 at 5:36 PM, Amar Tumballi <atumball@xxxxxxxxxx> wrote:

On Mon, Aug 27, 2018 at 8:47 AM, Amar Tumballi <atumball@xxxxxxxxxx> wrote:

On Wed, Aug 22, 2018 at 12:35 PM, Amar Tumballi <atumball@xxxxxxxxxx> wrote:

Hi All,

Below is an update about the project’s move towards using clang-formatter for imposing few coding-standards.

Gluster project, since inception followed certain basic coding standard, which was (at that time) easy to follow, and easy to review.

Over the time, with inclusion of many more developers and working with other communities, as the coding standards are different across projects, we got different type of code into source. After 11+years, now is the time we should be depending on tool for it more than ever, and hence we have decided to depend on clang-formatter for this.

Below are some highlights of this activity. We expect each of you to actively help us in this move, so it is smooth for all of us.

• We kickstarted this activity sometime around April 2018
• There was a repo created for trying out the options, and validating the code. Link to Repo
• Now, with the latest .clang-format file, we have made the whole GlusterFS codebase changes. The change here
• We will be running regression with the changes, multiple times, so we don’t want to miss something getting in without our notice.
• As it is a very big change (Almost 6 lakh lines changed), we will not put this commit through gerrit, but directly pushing to the repo.
• Once this patch gets in (ETA: 28th August), all the pending patches needs to go through rebase.

All, as Shyam has proposed to change the branch out date for release-5.0 as Sept 10th [1], we are now targeting Sept 7th for this activity.

We are finally Done!

We delayed in by another 4 days to make sure we pass the regression properly with clang changes, and it doesn't break anything.

Also note, from now, it is always better to format the changes with below command before committing.

 sh$ cd glusterfs-git-repo/

 sh$ clang-format -i $(list_of_files_changed)

 sh$ git commit # and usual steps to publish your changes.

Also note, all the changes which were present earlier, needs to be rebased with clang-format too.

One of the quick and dirty way to get your changes rebased in the case if your patch is significantly large, is by applying the patches on top of the commit before the clang-changes, and copy the files over, and run clang-format -i on them, and checking the diff. As no code other coding style changes happened, this should work fine.

Please post if you have any concerns.

What rules does clang impose on function/argument wrapping and alignment? I somehow found the new code wrapping to be random and highly unreadable. An example of 'before and after' the clang format patches went in: https://paste.fedoraproject.org/paste/dC~aRCzYgliqucGYIzxPrQ Wondering if this is just me or is it some problem of spurious clang fixes.

Regards,
Ravi

Noticed some glitches! Stand with us till we handle the situation...

meantime, found that below command for git am works better for applying smaller patches:

 $ git am --ignore-whitespace --ignore-space-change --reject 0001-patch

-Amar

 

Regards,

Amar

 

[1] - https://lists.gluster.org/pipermail/gluster-devel/2018-August/055308.html

 

What are the next steps:

• The patch of adding .clang-format file will get in first
• Nigel/Infra team will be keeping the repo with all files changed open for review till EOD 27th August, 2018

This changes to 05th Sept, 2018

 

• Upon passing regression, we will push this one change to main branch.
• After that, we will have a smoke job to validate the coding standard as per the .clang-format file, which will vote -1 if it is not meeting the standard.
• There will be guidelines about how to setup your own .clang-format setup, so while sending the patch, it gets posted in proper format
  • This will be provided for both ./rfc.sh and git review users.
• Having clang-formatter installed would be still optional, but there would be high chance the smoke would fail if not formatted right.

Any future changes to coding standard, due to improvements in clang-format tool itself, or due to developers believing some other option is better suited, can be getting in through gerrit.

Also note that, we will not be applying the changes to contrib/ directory, as that is expected to be same as corresponding upstream coding standard of particular project. We believe that helps to make sure we can quickly check the diff with corresponding changes really easily.

Happy to hear any feedback!

Regards,
Amar (on behalf of many Gluster Maintainers)

--

Amar Tumballi (amarts)

--

Amar Tumballi (amarts)

--

Amar Tumballi (amarts)

--

Amar Tumballi (amarts)

_______________________________________________
Gluster-devel mailing list
Gluster-devel@xxxxxxxxxxx
https://lists.gluster.org/mailman/listinfo/gluster-devel

_______________________________________________
Gluster-devel mailing list
Gluster-devel@xxxxxxxxxxx
https://lists.gluster.org/mailman/listinfo/gluster-devel