注意Pull Requests中的隐形代码(译)

编程中我们处理的一般都是可见的修改:代码中的逻辑,声明的依赖,暴露的API。然而,还存在很多相关的隐形修改:传递依赖,代码生成,一些manifest 文件。

在code review中我们的注意力主要集中在可见的代码中,因为可见的代码显示在diff中:

在code review中注意隐形代码的变化同样重要。传递依赖或者代码生成可能使得编译后的二进制文件变大或者降低性能。Manifest文件有时会引起兼容性问题。这些问题往往在release阶段才能被发现。

在Cash Android组中经过几个这样的问题后,我们决定在pull request阶段提高隐形代码的可见性。


在每一次CI build中,我们计算二进制文件大小,方法的数量(Android app一个很重要 的度量)等数据,并把数据写入一个共享的存储中。当一个pull request 的 CI build 完成后,我们绘制出与先前build 数据的对比图表。数据会随comment一起发送给提交者,以保证代码提交者和reivewer可以注意到这些变化。

这些重要的度量使得代码的可见性增加,以保证结果与自己的预期一致。新增加的依赖是增加了10个方法还是 10000个方法,新增的图片是增加了20KB还是2MB,一目了然。


除了这些简单的数字统计,我们还捕获整个Application的依赖图,和gradle的build脚本。

这里我们把kotlin从1.0.4升级到了1.1,可以看到我们不仅减少了3个方法数,而且可以看到他们在kotlin-runtime增加了对于 JetBrains’ annotations的依赖。

Android build 使用manifest merger来创建Application中最终的manifest。它会merge我们用到的所有library中的manifest。这个合并的manifest是我们的app可以获取Android操作系统的所有公共API,定义了入口点、暴露的service和需要的权限等。由于manifest可能会在升级时引起兼容性问题,因此跟踪这个文件的变化非常重要。


Square’s的开源库 Whorlwind简化了在Application增加指纹支持的难度。它在manifest中声明了获取系统的指纹识别权限。当把这个library增加到Cash项目中时,这个权限声明就会被merge到最终的manifest中,因此就会显示在pull requst的comment中。

当然这个例子是我们预期的结果,有时一些意外的权限或者错误的暴露组件可能会被merge进去,引起升级问题或者安全漏洞。

并不是所有的隐含信息都应该在comment中暴漏,比如,方法数量的变化常常让我想看看方法的实际变化。但是方法数的变化常常是数以千计的,使得comment没有什么意义。我们把diff文件作为附件添加到CI build中,而不是直接在comment中展示method的diff。


当我对comment中的一些细节有兴趣时,只需要点击链接即可。这个例子展示了一次build中的一个shard提供了所有方法diff、一个ProGuard remove报告、一个资源压缩报告,一个Gradle Profile展示了编译速度,一个展示project module依赖关系的图片。一些其他的shard展示Android的 lint报告或者test执行的总结等等。

选择在comment或者CI中增加什么需要根据Project来决定。对于Cash Android来说,以上是我们认为比较重要的需要展示的。目标应该是一样的,使得隐形信息可见是非常重要的。

欢迎关注公众号wutongke,定期推送移动开发前沿技术文章:

wutongke

推荐阅读更多精彩内容

  • Android 自定义View的各种姿势1 Activity的显示之ViewRootImpl详解 Activity...
    passiontim阅读 127,592评论 18 546
  • Spring Cloud为开发人员提供了快速构建分布式系统中一些常见模式的工具(例如配置管理,服务发现,断路器,智...
    卡卡罗2017阅读 74,630评论 12 116
  • Gradle配置最佳实践 本文会不定期更新,推荐watch下项目。如果喜欢请star,如果觉得有纰漏请提交issu...
    Solang阅读 741评论 0 2
  • 这两天越来越觉得自己知识的匮乏,对于感悟,想些东西,无从下笔。对于昨天的感悟就写了两句话,收在了私密文章里,今晚...
    收子阅读 50评论 1 5
  • 下午放学以后,吴强让杜白去他家吃饭,被杜白找了个理由婉拒了,吴强想说什么,但终究还是没说。 杜白的父母在杜白读...
    皮皮貅阅读 120评论 0 0