记一次代码事故之后

[Elfe]

写在前面：近来管理的事儿做得多了些，也就多了些时间去思考什么是公司的“工程师文化”。所谓文化，一定是整个公司都浸淫着的氛围，得靠点滴小事才能折射出来。所以就开始随手记一些工作中的小事儿，给自己一些答案，放在这里也能和大家多一些交流。

---

某下午，大家正各自忙活着，忽然传来一声吼：“都别pull代码哈，master上的代码不知被谁revert成两个月前的了！”（没错，就是百姓网网站的代码库）。立刻，锁部署系统（后来验证部署系统还是有足够防范不会把老代码直接发布的，但当时那个紧张呀），手工恢复代码库，几个人折腾了快一个小时，才安定下来开始找元凶：是一个实习生小朋友犯迷糊想pull却打成了push --force，指向主repo的master。

几个人坐下来讨论今后该如何防范类似问题。
取消实习生的master写权限？这会严重影响实习生和他mentor的生产效率，不可取；更何况，正式员工也会犯错呀，是不是他们也要根据资历分出个三六九等呢？
那就做更多的mentoring、教会了才给权限？可这次新人的mentor可是公司里最著名的布道github工作流程的人，真的全都教过、教会了，新人只是一时迷糊而已。

那怎么办？

最终的方案：
1. 取消所有人的baixing repo的写权限，只能在自己的repo上修改。
2. 所有工程师要merge代码时就向一个robot发送merge命令。robot会进行各种校验（merge命令发起方的身份、是否有PR、PR是否有reviewer给出LGTM、接下来还考虑加UT等等），成功后才会用robot的账号执行merge操作。
3. 由于同学们已经习惯了在github上review代码然后点merge按钮了，我们就写了小段油猴脚本，把github PR下面那个merge按钮变成了给robot发merge命令。

于是，除了需要设置一下油猴插件，之前的工作方式几乎没有任何改变，堪称完美！

---

一场虚惊后，我们的工作方式又进化了一步。我们一直都信奉breaking things and moving fast，有很多地方为了效率我们会采取一些（以我之前在微软时的标准来看）简单粗暴甚至危险的方式。随着公司的成长，我们要学着避免breaking things，但如何在避免它的同时又不影响moving fast，是一个很值得研究的话题。今后应该不断会有case扔出来和大家交流。

---

5.28携程事故，左耳朵耗子说了段话特别有道理，结合着咱们自己的事情回顾、自夸一下 :P

技术上出故障是必然的。能否体现一个公司是技术公司，重要看这几点：1）故障的恢复是否有技术含量，2）公司对故障的处理方式，如果是通过加更多的流程，或是通过加更多的权限管控，或是通过处罚犯错误的人，或是上升到员工意识形态上，而不是去用更好的技术来解决，那么这个公司不会是个技术公司。

背景：

• 我们用的是github的private仓库，没有禁force push的选项。
• 我们确实要求fork->edit->pull request->code review->merge，但我们认为所有同学都应该有权限review并merge他熟悉的那部分代码，所以之前开放了write权限。
• 我们的工程师客户端工具百花齐放，幸好code review都一样在github网页上进行，因此油猴脚本是对现有工作方式影响最小的方案。

看到很多同学都在说gerrit，我开始考虑让我们的一些小项目尝试下。找到了gerrithub能把gerrit和github结合起来这里是介绍。不知有同学了解么？或者我们应该自己搭gerrit？

看到昨天 github 的 blog 上说 马上就要有 protected branches 的功能 。
这样一来我们的油猴脚本确实就多余了。
不过 anyway ，借着这个机会重新强调了 code review ，并从形式上强制执行，也是收获。

最后的更新：
Github 上 protect branch 的功能我们已经用起来了，原来的油猴脚本光荣退休。

=========
以下摘自实现 github 强制上 code review 功能的同学发出来的 how it works 邮件：

Github 提供了 webhook ，就是当 repo 发生变动时，可以触发一个 web 调用，填入自己的网址后 github 就会把变动事件的信息发到那边去，比如有人评论了，新建 pr 了，等等： https://developer.github.com/webhooks/

而改变 merge 按钮的那个地方的状态检查，则是 github api 的一个功能： https://developer.github.com/v3/repos/statuses/，可以用来标记此 pr 的状态，一般可以有语法检查， UT 等，我们这次只加在了检查代码 review 情况上，语法后 UT 后面陆续加。

然后 Github 最近又增加了，一定要某个检查的 status 是通过的情况下才允许合并 pr 的功能。

于是结合以上三者，当有 pr 新建时，对该 pr 打一个需要 reivew 的 status ，当有人 lgtm 了以后再打上通过的 status ，并制定这个的 status 一定要是通过的情况下才允许合并 pr ，就 OK 了。

[feiyuanqiu]

你们的开发可以直接向 master push？
我们公司是用gerrit做管理，有两道审核，我提交之后让组长审了才进入develop分支，然后要发布了让主管合并到 master

[XiaoxiaoPu]

不能禁止在 master 上 forch push 吗？

[9hills]

我司要求所有提交必须过code review

[gDD]

@XiaoxiaoPu 看楼主描述他们用的 GitHub，无法开启禁止 force push 选项。

[humiaozuzu]

@gDD Github Enterprise 的 org admin 里面可以配置

[Elfe]

@9hills 我们也要求。只不过是靠自觉。这次加的robot会检查PR下面有没有别的reviewer回复LGTM(looks good to me)

@XiaoxiaoPu 嗯，我们用github，并且希望所有工程师都能merge代码到主干，而merge代码和push用的是同样的写权限，所以……

@feiyuanqiu 我们是工程师代码经同伴review过、unit test跑过、自己线下环境测过，就可以自己merge、自己点deploy按钮了。一天deploy几十次的。 :D:D 这叫moving fast，确实会breaking things，但到目前来看收益大于损失。

[dalang]

为耗子的评论点赞

[Elfe]

@humiaozuzu org 配置里，只有 read/write/admin 三种权限。如果禁了push，等于禁了merge。
所以这次是禁merge，但通过一个robot加一段脚本来达到原来一样的效果。

[mintist]

同Gerrit无法直接push master，一定要至少一个人Review通过才可以

[tracyone]

哈哈，历历在目的感觉....好好玩

[ChiangDi]

感觉好像把问题搞复杂了，像 github 那样只让 fork 和发 pr 就好了。

[clino]

gerrit +1

[Elfe]

@ChiangDi 没错啊，我们正常的流程就是要fork，改代码，然后发pr，merge。正常流程是绝对不会出问题的。但是，既然每个人都有往主repo的master写入的权限，那就有可能脑抽敲出push --force baixing master这样的命令啊。

[humiaozuzu]

@Elfe 在 org 页面点 admin tools -> admin 就有

[maxiujun]

应该还是没玩明白

[ChiangDi]

@Elfe 看看楼上，github 可以直接禁止 push -force

[kaneg]

我们公司也出现过类似的事，有个人不知道对master做了什么操作，所有文件（近5万个）的历史只有他一人的一次提交了。或许历史还有，但IDEA的git插件里就看不到文件的修改历史了

[zhuang]

以前遇到过类似的事情，现在来看还是个“流程”问题。



Github workflow 强调 master 就是可部署版本。

所以我更倾向于限制 master 的写权限。

在 master 之外建立 test 分支，可以让所有人对 test 可写，此分支的主要用途在于 CI。

只有 CI 每日构建成功的版本才会定期合并到 master，当日构建失败或者发生重大事故可以由 master 恢复，也可以无条件回滚至前一日构建版本上。

还是要坚持 Issue-PR-Merge 流程，去掉前两步看似走得快了，但是步子太大，容易扯着蛋。



我不清楚机器人账号执行写操作的具体实现是什么，但我想到几点由此带来的问题：

需要另一套日志系统记录提交给机器人的指令，或者将相关信息写入 commit log 当中。两套方案各有利弊。

github 更新可能会破坏现有的工作流程。

最后也是最重要的一点：

仍旧不能避免“推送了一个不可部署的版本到 master 分支”的问题。

[kzzhr]

对实习生真关心啊，还找人不 :scream:
之前也听L大夸过一次好，这次感觉就是叼

[benjiam]

没有gerrit 根本没有解决问题，差评。

[skydiver]

问题很常见，解决方法好奇葩。
直接在master分支上禁止force push不就可以了。
另外用油猴脚本来hack这方法也太不靠谱了。。检查可以在ci系统里做啊。github都有对应的api。

[axb]

类似的问题，我们用的是gitlab。

做法是修改了gitlab，区分merge和push权限，把所有人直接push master（包括-f）的权限取消，只能通过merge合并代码，并且做了类似Gerrit的权限控制，只有review之后的代码才能合并。

[xi_lin]

master分支禁止force push才是正道。。

[clippit]

油猴脚本这种方法感觉不好，要求客户端配置，流程不收敛，最好是在服务端配置好呢

[Elfe]

@humiaozuzu @ChiangDi 禁止 force push 是github enterprise才有的特性？我们使用的是普通的github org 有private repo的那种账户，在organization profile里没有找到admin tools哎。

搜到了这个 http://stackoverflow.com/questions/5094524/github-prevent-collaborators-from-using-push-f 今年6月8号github stuff的回复，disable force push还只是在wish list上……

[Elfe]

另外就是我们也考虑过webhooks，但似乎没能找到pre-push之类的event https://developer.github.com/webhooks/

[holulu]

难道 GitHub 上还没有分支保护功能？我看 coding.net 上有这个功能，启用之后可以指定哪些项目成员才能更新特定分支。

[lujiajing1126]

只允许fast forward不就好了？没必要搞这么复杂吧

[sinux]

我们连分支和git flow 都不能push到远程，开分支要申请......

[humiaozuzu]

@Elfe 这么大的企业为什么不自己 host 呢

[blues9]

用gerrit吧，安全可靠

[robertlyc]

现在的软文好可怕

[realpg]

我觉得楼主公司使用github的姿势不正确。

我没用过github的private服务，我还不知道他们的权限管理体系是啥样的。我这边正式项目用git的不多，如果用git，使用的是我曾经的一个git大牛同事自建的一套类似github的系统（PS 经过授权我可以带到别的公司去搭建使用），我们的套路是员工双账号，开发账号fork出来自己的库，只能发pull request，然后由另外一个任何有权做code review的人开浏览器隐身模式登陆有权限账号进行final code review，如果code review通过则merge进申请的分支。

就算你是CTO亲自写的代码，你也得经过别人的final code review才能进主库，任何时候包括线上紧急BUG修复也不允许自己写程序自己merge

自己merge是最大的大坑；任何人都会有一种自我感觉良好。

[jiyee]

好软文，能引起共鸣，又宣贯了价值观。

[jiaozi]

为什么感觉你们提交代码的流程都如此繁琐。。。难道因为我们用了比较low的svn。反正我们该提交提交，该合并合并的。至今还木有出现过事故。希望以后也不会。。。

[wbsdty331]

坐等“我就是那个用了push -force的员工”

[w88975]

我靠 master分支 实习生都能有权限?
我司的所有repo,都要管理员创建,fork出来已pull request的方式来提交,没出过事故.

[w88975]

某次发现主版本不小心给我赋予了权限,都吓的心惊胆跳的,马上找管理员给我取消掉,经常用cli,一不小心推送到origin分支就惨了...

[Phariel]

第一 大家都有master的操作权限简直就是作死 master仅限于代码层层通过pull request review才能被系统自动merge
第二 新人为什么要force操作 有人敢加force的一律拍死 (误

[pyKun]

楼主用下gerrit，git review的功能很合适。。。

[cxshun]

实习生可以有权限直接推master？要作死也不用这样啊。
本来在管理上面就是有三六九等，不要不承认，公司里面就是这样。你给一个新来的员工一个管理员试试。平等？这个世界从来都没有过。

push force，用git到现在还没试过这样的，实习生真的知道这是啥意思？啥都不管，直接force，这真的是程序员应该有的做法。

[Andiry]

你说pull打成push我也就信了，打成push --force？难道你家实习生用的是pull --force？

[Elfe]

@Livid 为啥我这条消息不能作为附言发出？
答楼上各位

我们是要求fork出自己的repo、建自己的branch，commit后发pull request，然后code review/merge 到主repo的master。这个流程正常操作是绝对不会有问题的，只是没按规则出牌就出了意外。

出意外的原因是为了让所有工程师都能review完别人的代码后merge（我们认为只又少数人有code review和merge代码的权限很不合理，会严重降低工作效率），就必须给大家开write权限（我们用的是普通的github private，只有read/write/admin三种权限设置），从根上无法禁止直接push（包括force push）。各位自建git、用github enterprise的同学的建议都很靠谱， 可惜我们目前做不到。

确实也有考虑过是不是该强制大家用同样的git工具遵循同样的操作方式，来杜绝意外的发生。不幸，百姓网对工具的使用向来不做限制，甚至可以说是鼓励大家小范围尝鲜，导致百花齐放（其实我对这个是有吐槽的，以后再展开）。大家与git相关的玩法很多，大概只有通过github网页上code review PR并merge，这一点是一致的。所以这次就通过用油猴脚本让大家能继续“merge”，来确保避免breaking things的同时没有妨碍moving fast.

关于code review，我们以前一直是要求“必须”，但全凭自觉。过去多年一直执行得挺好的，但最近一年感觉松懈了。所以这次油猴脚本加了对code review得强制要求。接下来会去研究一下gerrit，看是否值得为此改变大家得工作习惯。多谢大家建议！

[Elfe]

@humiaozuzu 两年多前我们开始启用github时不过二、三十号技术人吧。现在把数据之类的全都算上也就五、六十来技术FTE，做主站开发的不过十几个，不算大。

@Andiry 是啊，我们也都很不理解，纷纷问xxx你用pull --force是想干嘛呢……还是没有教好呀

@kzzhr 招呀，来吧 http://www.lagou.com/gongsi/j9343.html

嗯，我挺乐意写这样的软文的，这样的交流总有收获。我以前还真没有留意gerrit，这次那么多人提，是得认真看一下了。先谢过哈。

[lovedboy]

pull request + 1

[MarioLuisGarcia]

--force是能乱用的么

[hitmanx]

我们也是用gerrit+code review才能push到master上去

[oxoxoxox]

gerrit + 1
不然始终是一个安全隐患

[Mark24]

先MARK

[infun]

我携。。。不说了

[juicy]

别的分支不code review还可以理解，线上分支不管怎么都得设置防线~

即便一个人出错的概率只有1%，100个人出错的概率就能达到6成了。。

[feiyuanqiu]

我昨天在回帖之后又思考了下，作为一个下层开发人员，我对权限控制还是很认同的，有什么能力享受什么待遇就拥有什么权限和责任。
事实上很多人其实是没有能力（也不愿意）承受那么大的权限的，直接把权限给他是对他的一种压力，就像一个人才拿到驾照你就把一辆卡车交给他去开一样，这其实是把拿更多薪水担任更核心角色的人的任务和责任分摊给了下层开发人员。不能主观地觉得只要他按照流程来就不会有问题，而应该客观地认识到每个人都会犯错，制度、管理的作用就是限制每个人犯错的时候能造成的危害。
而主帖中你提到的好处是开发效率，但是实际工作中一次生产环境的事故就可以毁掉几个月辛苦工作的成果（公司形象、项目绩效、给高层的印象等等），是非常打击士气的

对于你提到的左老师的观点，我认为技术是在完善的管理下用来提升效率的，而不是用来堵管理上的漏洞的

[LPeJuN6lLsS9]

假如push -f后“丢失”的commit可以花一分钟轻松找回呢？还需要这些限制吗

[fuyufjh]

在巨硬就没有这种问题……作为实习生我啥权限都没有（摊手

[hdshen]

master 分支的权限不是谁都给的。。

开发用自己的dev分支

合并到主分支 必须 request 过代码审核

[genffy]

看到百姓网就会想到 jonhax呢，。。。。。

[bdnet]

gitflow + 禁止 force push 主要分支，

code review 用 merge request （gitlab）

持续集成和部署 Jenkins ，后期会考虑用 gitlab ci + docker 做持续集成

50以下的小团队应该完全没问题（现在20人小团队无鸭梨）。

[Jaylee]

git 工作流有问题啊，rd不应该直接有master的权限的

[zzwangsh]

没有所谓规范正常必须的规范流程去针对所有公司的代码更新流程。适合自己的就是最好的。

[merlinran]

呵呵，这样的错误我也犯过。把pull打成push，再--all -f合用，酷毙了。还是远程团队，赶紧邮件周知。老大说，这傻事谁都干过，注意就行了。这跟rm -rf神似。

[mozartgho]

用SVN就好了，哪会有这些乱七八糟的问题。我们开发文档上定义了一套SVN的Best Practice，code review也很方便啊！觉得SVN完全满足我们的开发流程需求，git的优点还不足以让我们换掉SVN，就这些！

[Elfe]

@Livid 报bug：附言太长了发不了，里面的markdown没转换……问题本身倒不大，不过你好歹提醒一下么……

[Livid]

@Elfe 附言的 Markdown 转换功能今天会有，抱歉！

[Livid]

cc @Kai

一会测试。

[Kai]

@Livid 好的

[Elfe]

由上面gerrit+github的文章顺藤摸瓜找到cloudfoundry的实践 http://blog.cloudfoundry.org/2012/04/11/the-new-cloudfoundry-org-gerrit-jenkins-github/ 不过也是3年前的了。不清楚他们是自己搭的gerrit还是用的某public服务。

gerrit+github的服务我只找到GerritHub.io, 可是从他们的blog来看，似乎他们和github关系一点都不紧密，要用还是不太放心。 http://gitenterprise.me/2015/06/25/github-api-change-causes-problems-to-jenkins-and-gerrit/

有实践gerrit+github方案的同学么？传授一点经验？ @mintist @clino @benjiam @yangshengwu @pyKun @hitmanx @oxoxoxox

[pyKun]

@Elfe

http://docs.openstack.org/infra/manual/developers.html

可以参考这个

[clino]

@Elfe "gerrit+github方案"是什么意思啊?
我感觉有了gerrit就不需要github了哈

[krafttuc]

@elfe gerrit 是有 GitHub 插件的，以下是连接

https://gerrit.googlesource.com/plugins/github/+/master/README.md https://www.packtpub.com/books/content/using-gerrit-github

[Elfe]

@clino 因为我们目前全部都在github上，没有特别理由不想换啊

@pyKun @krafttuc 好，我去看看。多谢！

[clino]

@Elfe 那就gerrit可写,github对用户来说只读,gerrit相关分支push到github用工具自动进行

[Livid]

TopicSupplement 的 Markdown 支持已经部署。

谢谢 Elfe。

[vibbow]

https://pic.vsean.net/di/T3WI/QQ截图20150810023248.png
禁止force push不是应该是git服务器的必备功能么？

[vibbow]

https://pic.vsean.net/di/PMVJ/QQ截图20150810023440.png
这种功能github应该也有的把

[fly2never]

Atlassian Stash是一个不错的私有方案, 支持分支级的合并策略

[tonitech]

除了PM，其他人都没有权限像master分支push代码，实习生能做这种事说明你们没有做protect branch啊。

[kisnows]

我都没有权限忘master分支上push，都是push到一个新分支，然后给老大提和并请求。
否则估计我们公司也出问题了，因为手贱或者犯迷糊有的时候也会直接push到master上。还好reject了。
所以实习生还是不要给mater的写权限了

[Anybfans]

反正我们正式网站是Masterfen分支。
大家开发全是develop分支or 其他分支，
测试网也是develop分支，到时候发布大网

[gDD]

@fly2never
@tonitech
@kisnows 非企业版的 GitHub 是没有你们所述的功能的。。。

[wych]

还是权限放的太开了。
耗子的话有道理，但是前提是管理上有合适的流程啊，不能因噎废食。

[tonitech]

@gDD 你说的对，我公司的gitlab确实有没有这个功能，我的做法是不告诉下面的小朋友他们有权限push到master的，所以他们都觉得自己无法做合并代码push到master的工作。然后这个合并代码发布的工作只给PM，所以从来没有闲杂人等去动master的代码。

[Elfe]

Github 新增了 protect branch 的功能，我们现在用起来了。原来的油猴脚本光荣退休。