记一次代码事故之后

2015-08-02 22:29:21 +08:00
 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)公司对故障的处理方式,如果是通过加更多的流程,或是通过加更多的权限管控,或是通过处罚犯错误的人,或是上升到员工意识形态上,而不是去用更好的技术来解决,那么这个公司不会是个技术公司。

13597 次点击
所在节点    程序员
82 条回复
cxshun
2015-08-03 11:46:00 +08:00
实习生可以有权限直接推master?要作死也不用这样啊。
本来在管理上面就是有三六九等,不要不承认,公司里面就是这样。你给一个新来的员工一个管理员试试。平等?这个世界从来都没有过。

push force,用git到现在还没试过这样的,实习生真的知道这是啥意思?啥都不管,直接force,这真的是程序员应该有的做法。
Andiry
2015-08-03 11:57:15 +08:00
你说pull打成push我也就信了,打成push --force?难道你家实习生用的是pull --force?
Elfe
2015-08-03 12:07:44 +08:00
@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
2015-08-03 12:17:13 +08:00
@humiaozuzu 两年多前我们开始启用github时不过二、三十号技术人吧。现在把数据之类的全都算上也就五、六十来技术FTE,做主站开发的不过十几个,不算大。

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

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

嗯,我挺乐意写这样的软文的,这样的交流总有收获。我以前还真没有留意gerrit,这次那么多人提,是得认真看一下了。先谢过哈。
lovedboy
2015-08-03 12:18:57 +08:00
pull request + 1
MarioLuisGarcia
2015-08-03 14:23:01 +08:00
--force是能乱用的么
hitmanx
2015-08-03 17:22:58 +08:00
我们也是用gerrit+code review才能push到master上去
oxoxoxox
2015-08-03 17:29:16 +08:00
gerrit + 1
不然始终是一个安全隐患
Mark24
2015-08-03 17:31:16 +08:00
先MARK
infun
2015-08-03 17:34:40 +08:00
我携。。。不说了
juicy
2015-08-03 17:57:56 +08:00
别的分支不code review还可以理解,线上分支不管怎么都得设置防线~

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

对于你提到的左老师的观点,我认为技术是在完善的管理下用来提升效率的,而不是用来堵管理上的漏洞的
LPeJuN6lLsS9
2015-08-03 19:08:29 +08:00
假如push -f后“丢失”的commit可以花一分钟轻松找回呢?还需要这些限制吗
fuyufjh
2015-08-03 21:59:22 +08:00
在巨硬就没有这种问题……作为实习生我啥权限都没有(摊手
hdshen
2015-08-03 22:02:04 +08:00
master 分支的权限不是谁都给的。。

开发用自己的dev分支

合并到主分支 必须 request 过代码审核
genffy
2015-08-03 22:11:05 +08:00
看到百姓网就会想到 jonhax呢,。。。。。
bdnet
2015-08-03 22:15:30 +08:00
gitflow + 禁止 force push 主要分支,

code review 用 merge request (gitlab)

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

50以下的小团队应该完全没问题(现在20人小团队无鸭梨)。
Jaylee
2015-08-03 22:24:24 +08:00
git 工作流有问题啊,rd不应该直接有master的权限的
zzwangsh
2015-08-04 09:41:36 +08:00
没有所谓规范正常必须的规范流程去针对所有公司的代码更新流程。适合自己的就是最好的。
merlinran
2015-08-04 10:15:59 +08:00
呵呵,这样的错误我也犯过。把pull打成push,再--all -f合用,酷毙了。还是远程团队,赶紧邮件周知。老大说,这傻事谁都干过,注意就行了。这跟rm -rf神似。

这是一个专为移动设备优化的页面(即为了让你能够在 Google 搜索结果里秒开这个页面),如果你希望参与 V2EX 社区的讨论,你可以继续到 V2EX 上打开本讨论主题的完整版本。

https://www.v2ex.com/t/210303

V2EX 是创意工作者们的社区,是一个分享自己正在做的有趣事物、交流想法,可以遇见新朋友甚至新机会的地方。

V2EX is a community of developers, designers and creative people.

© 2021 V2EX