[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
<https://about.me/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