[Gluster-infra] Clang-format change postmortem (12th Sept, 2018)

Sankarshan Mukhopadhyay sankarshan.mukhopadhyay at gmail.com
Thu Sep 13 06:09:25 UTC 2018

On Wed, 12 Sep 2018 at 19:56, Amar Tumballi <atumball at redhat.com> wrote:

> People Involved
>    - Nigel
>    - Amar
> <https://hackmd.io/_zKtoPXUT7mDAyV6OxzZRQ?both#Timeline-of-events-in-IST>Timeline
> of events (in IST)
> 1725 - Nigel merges Amar’s patch with the .clang-format file
> 1727 - Nigel lands the .clang-format changes to master as gluster-ant.
> Smoke jobs pass at this point)
> 1746 - Amar notices that some files are missing in the clang patch.
> 1752 - Nigel lands a new patch with the missing files (all .c files)
> 1811 - Amar notices compilation issues after landing the .c changes
> because it modifes files with the pattern -tmpl.c. Amar starts working on
> a fix.
> 1839 - Nigel notices that the Jenkins job for clang-format doesn’t fail
> when it’s supposed to fail and goes to fix.
> 1855 - Clang-format Jenkins job is now fixed.
> 1906 - Amar’s fixes are merged with manual votes for Smoke and Centos
> Regression from the Infra team. At this point, the builds were passing, but
> we had voting issues
> <https://hackmd.io/_zKtoPXUT7mDAyV6OxzZRQ?both#What-Went-Wrong>What Went
> Wrong
>    - We staged the changes on Github on Sept 10th. Given the size of
>    changes, we we missed that the command used to make the changes only caught
>     .h files and not .c files. The following is the command in question.
>    find . -path ./contrib -prune -o -name '*.c' -or -name '*.h' -print |
>    xargs clang-format -i
>    - With the changes that we landed, we did run into build bugs 1
>    <https://review.gluster.org/#/c/21130/>, 2
>    <https://review.gluster.org/#/c/21128/> and fixed them. However, we
>    did not verify that all the files were in fact modified or sync up on the
>    find command.
>    - We had a general framework of agreement on the steps to do but we
>    looked at it as a code change rather than an infrastructure change. There
>    wasn’t a well defined go/no-go checklist.
Thank you for writing this up and sharing. The “infrastructure piece” part
is a great realisation.

Can we start to use “retrospective” instead of post mortem?

>    -
>    - In the middle of this, we had a freebsd-builder enabled that made
>    the smoke job for the final fix not vote. This needed a manual vote from
>    the Infra team.
> <https://hackmd.io/_zKtoPXUT7mDAyV6OxzZRQ?both#What-Went-Well>What Went
> Well
>    - We did reasonably good planning to find potential issues and in
>    fact, did find some potential issues.
>    - Nigel and Amar were on hand and available to fix any potential
>    issues that popped up
>    - The changes landed at the end of a working day for India the day
>    before a public holiday. While there was impact, it was much less than a
>    similar change performed at working hours.
> <https://hackmd.io/_zKtoPXUT7mDAyV6OxzZRQ?both#Future-recommendations>Future
> recommendations
>    - Template files need to be caught by the clang-format job correctly
>    so that they are not checked for formatting. Or the name of the file needs
>    to be changed so that they don’t end with .c or .h.
>    - In the future, high impact changes need a good process which has at
>    least an acceptance criteria, go/no-go checklist, and a rollback procedure.
>    - This work is currently incomplete and the bug3
>    <https://bugzilla.redhat.com/show_bug.cgi?id=1564149#c39> tracks the
>    remaining action items.
> -----
> Thanks Nigel for the postmortem report.
> --
> Amar Tumballi (amarts)
> _______________________________________________
> Gluster-infra mailing list
> Gluster-infra at gluster.org
> https://lists.gluster.org/mailman/listinfo/gluster-infra

sankarshan mukhopadhyay
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.gluster.org/pipermail/gluster-infra/attachments/20180913/e3c82c2c/attachment.html>

More information about the Gluster-infra mailing list