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

[Amar Tumballi]

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.
   - 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)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.gluster.org/pipermail/gluster-devel/attachments/20180912/f39512e0/attachment.html>