把Code Review变成一种开发文化而不仅仅是一种制度
把Code Review 作为开发流程的必选项后,不代表Code Review这件事就可以执行的很好,因为Code Review 的执行,很大部分程度上依赖于审查者的认真审查,以及被审查者的积极配合,两者缺一不可!
如果仅仅只是当作一个流程制度,那么就可能会流于形式。最终结果就是看起来有Code Review,但没有人认真审查,随便看下就通过了,或者发现问题也不愿意修改。
真要把Code Review这件事做好,必须让Code Review变成团队的一种文化,开发人员从心底接受这件事,并认真执行这件事。
要形成这样的文化,不那么容易,也没有想象的那么难,比如这些方面可以参考:
- 首先,得让开发人员认识到Code Review这件事为自己、为团队带来的好处
- 然后,得要有几个人做好表率作用,榜样的力量很重要
- 还有,对于管理者来说,你激励什么,往往就会得到什么
- 最后,像写自动化测试一样,把Code Review要作为开发任务的一部分,给审查者和被审查者都留出专门的时间去做这件事,不能光想着马儿跑得快又舍不得给马儿吃草
如何形成这样的文化,有心的话,还有很多方法可以尝试。只有真正让大家都认同和践行,才可能去做好Code Review这件事。
参考文章:https://zhuanlan.zhihu.com/p/73809355
自动化工具及规则
前端代码规范
eslint
config- coding里增加eslint的扫描
扫出来问题很多:后端也在改,每个迭代修改几类。这个需要大家看看是否参考后端的方式,定期梳理一批
提交MR前的必要条件
- 先设计再编码
建议大家在做复杂功能设计之前可以内部先简单进行一轮头脑碰撞,看一下是否思路可行,或者大家可以一起看一下是否有更好的实现方案。否则review的时候大部分逻辑都写完了,如果有更好的方案,不一定能在review的时候再提出修改了,就算提出了,也可能会涉及大量代码的重写,最终耽误工期。
- 本地代码通过
eslint
审查
在提交代码前,请确保error
、warning
都已处理完毕。有必要忽略eslint校验时可以考虑使用
/* eslint-disable */
/* eslint-disable no-param-reassign */
// eslint-disable-next-line
,
否则肉眼的力量是有限的
- PR要小
在做Code Review的时候,如果有大量的文件修改,那么Review起来是很困难的,但如果PR比较小,相对就比较容易Review,也容易发现代码中可能存在的问题。
有必要的话,可以拆分功能点,分阶段提交MR,提交的时候标注仅review还是review+merge
- 开发要预留时间,你的Reviewer也要提前周知预留时间
如何review
对评论进行分级
(感觉这个建议很好,原文出自:https://zhuanlan.zhihu.com/p/73809355)
在做Code Review时,需要针对审查出有问题的代码行添加评论,如果只是评论,有时候对于被审查者比较难甄别评论所代表的含义,是不是必须要修改。
建议可以对Review的评论进行分级,不同级别的结果可以打上不同的Tag,比如说:
- [blocker]在评论前面加上一个blocker标记,表示这个代码行的问题必须要修改
- [optional]:在评论前面加上一个[optional]标记,表示这个代码行的问题可改可不改
- [question]:在评论前面加上一个[question]标记,表示对这个代码行不理解,有问题需要问,被审查者需要针对问题进行回复澄清
类似这样的分级可以帮助被审查者直观了解Review结果,提高Review效率。
文件结构的检查
- 是否符合代码工程目前形成的常用文件结构
- 文件命名规范
- 文件放置的位置是否合理(比如demand的组件不能放到teamspace下)
js的书写规范
依据可查看文档:http://coding.jd.com/jd-common/js-style/
书写风格
变量
驼峰式命名
let cardList; let cardListButton; function getCardList(){}
常量
全部大写,使用下划线来分割单词
const TIMEOUT = 10000; const MAX_LENGHT = 10;
Function
对应的方法应该使用对应的动词,例如:
get/set
, add/remove
, create/destroy
, start/stop
, insert/delete
, begin/end
;
驼峰式命名;
构造函数的函数名,采用首字母大写(InitialCap);其他函数名,一律首字母小写。
css规范
命名规范:BEM + Utility-first
No全局污染
<style lang="less">
这种不带scoped
标签下的样式是全局生效的,慎写影响面积广的样式,比如
- 模块中禁止书写第一类,标签选择器
html,body{} div{} ul{} input{}
- 减少对通用样式的修改
.el-menu{} .jacp-header{} // 如需修改需要加前缀修饰,以免影响他处 .card-list .el-menu{}
No过度修饰的选择器
过度修饰的选择器并不理想。
过度修饰的选择器是指像 div.promo
这样的。很可能你只用 .promo 也能得到相同的效果。当然你可能偶尔会需要用元素类型来修饰 class
例如你写了一个 .error 而且想让它在不同的元素类型中显示效果不一样,例如
.error{ color:red; } div.error{ color: pink;} span.error{ color: pink; padding:14px; }
,但是大多数时候还是应当尽量避免。
再举一个修饰过度的选择器例子,ul.nav li a{}
。如前文所说,我们马上就可以删掉 ul 因为我们知道 .nav 是个列表,然后我们就可以发现 a 一定在 li 中,所以我们就能将这个选择器改写成 .nav a{}。
No浏览器前缀
无需手写浏览器前缀,因为我们是通过打包的时候处理的,autoprefixer了解一下
// 源码中只需书写 ::placeholder { color: gray; } // 打包后会自动加上前缀 ::-webkit-input-placeholder { color: gray; } :-ms-input-placeholder { color: gray; } ::-ms-input-placeholder { color: gray; } ::placeholder { color: gray; }
合并重复代码
如果有重复的样式需要书写的时候,请不要直接拷贝粘贴到其他位置,尽量合二为一,不同的部分再行单独处理,减少代码冗余。
// 以下代码 .el-upload{ display: none; } .el-upload-list__item{ display: none; } // 请合并成为 .el-upload, .el-upload-list__item{ display: none; }
合理使用HTML标签
语义化:请不要通篇div
Bad Semantics
<div class="article"> <div class="article_title">这里是标题</div> <div class="the_content">这里是内容 <div class="darkbold">这里加粗</div> 然后是普通文字.</div> </div>
Good Semantics
<article> <h1>这里是标题</h1> <p>这里是内容 <b>这里加粗</b> 然后是普通文字.</p> </article>
选择功能恰当的标签
- 如需行内元素可以使用
<span>
, 而不是设置成inline的<div>
- 图标icon使用
<i class="icon-add">
, 不要使用<span>+</span>
- 给img加上alt: 让禁用图片或者使用特殊设备的用户无障碍得了解你的网页信息,并且对图像搜索引擎友好
书写HTML标签注意
- 保持标签闭合
- 保持标签名小写
- 元素对齐,适当缩进
属性书写顺序
[建议遵循以下顺序] 同一 rule set 下的属性在书写时,应按功能进行分组,并以 Formatting Model(布局方式、位置) > Box Model(尺寸) > Typographic(文本相关) > Visual(视觉效果) 的顺序书写,以提高代码的可读性。
- 布局定位属性:display / position / float / clear / visibility / overflow
- 自身属性:width / height / margin / padding / border / background
- 文本属性:color / font / text-decoration / text-align / vertical-align / white- space / break-word
- 其他属性(CSS3):content / cursor / border-radius / box-shadow / text-shadow / background:linear-gradient …