跳转到: 导航, 搜索

Fuel/代码审查规则

为什么我们需要代码审查?

代码审查对于可持续的软件开发流程至关重要。它服务于以下目标

  1. 确认提议的代码更改适合其目的:它解决了陈述的问题并遵循了约定的架构。
  2. 分析代码更改的外部影响:它不会引入新的问题,不会降低整个解决方案的稳定性和性能。
  3. 强制执行代码质量标准:提议的更改结构良好、文档完善,并且与周围代码的编码风格一致。
  4. 传播有关架构和实现细节的知识,让其他开发人员了解代码库的当前状态。

提交你的代码进行审查

  1. 确保你的提交消息充分描述了提议的代码更改,并且符合提交消息指南
  2. 确保你的代码提高了周围代码的整体质量,避免快速而粗糙的解决方法,遵循代码质量标准
  3. 记住,获得其他开发人员的审查和批准是你的责任。积极主动并及时响应,使用 IRC 和邮件列表来引起核心开发人员对你的审查请求的注意,并尝试及时解决所有审查意见。

审查他人的代码

  1. 要友善。将你的评论表达为有帮助的建议。
  2. 在审查之前,了解代码编写的原因:这段代码是为解决什么确切的问题/功能而编写的?
  3. 了解代码的使用场景、数据流。
    • 例如:节点代理的审查请求。使用场景:代理在启动时从 rc.local 运行,它从命令行检查 nailgun url 并向 API 发送 POST 请求,发布从 ohai 获得的数据。
  4. 代码中是否解决了这些使用场景?
    • 例如:检查代理是否会在启动时运行,是否会收集数据并发送 POST 请求
  5. 确保代码不违反文档化的架构。对架构的更改应在蓝图和邮件列表中讨论。
    • 例如:部署编排器缺少有关节点的一些数据,这些数据可以轻松从 Nailgun API 获得,而初级开发人员使用 GET 请求从 Nailgun API 获取数据。
    • 审查者评论可以是:“编排器通过 AMQP 获取所有数据,不应该从 Nailgun API 获取任何数据。编排器必须在不使用 Nailgun 的情况下工作,因此我的建议是扩展 Nailgun 代码,以便通过 AMQP 发送所需的数据,以及其他数据。”
  6. 拿一张纸写下负面场景。代码是否处理了所有这些场景?它是否以处理即使现在不一定会发生但有可能发生的错误的方式编写?
    • 例如:代理 - 如果 ohai 没有提供某些数据怎么办?如果由于临时网络连接问题而 POST 请求失败怎么办?如果对象已经在后端创建,并且 POST 返回错误怎么办?代理是否会在建立网络连接之前启动?URL 不在命令行中的任何情况?
  7. 确保存在单元测试并覆盖
    • 蓝图或错误中定义的使用场景
    • 负面场景
    • 如果有其他要求,例如性能/可扩展性/线程安全性,也请检查特定的测试。
  8. 检查是否需要集成测试,或者是否计划进行集成测试。如果新代码影响到部署编排器的 RPC 调用:提供新数据或创建新的方法调用,那么要求进行集成测试是一个很好的理由。

合并提交

合并到 fuel-library 仓库的策略现在更加严格。核心审查者可以在满足以下条件时批准提交合并

  1. 至少有一位 SME(主题专家)表示 +1
  2. 至少有一位其他审查者表示 +1
  3. 如果补丁复杂、可疑或引入对参考架构的更改,则另一位核心审查者表示 +2

提交消息指南

git 提交消息的简短清单

  1. 提交消息的第一行是更改的简短(50 个字符)摘要,并与消息的其余部分之间用空行分隔。
  2. 提交消息包括对相关错误和蓝图的一个或多个引用。
  3. 提交消息的正文包括
    • 理由(例如,对参考错误的根本原因的解释)
    • 范围(此更改解决了或未解决参考错误或蓝图的哪些部分)
    • 更改对架构、安全、可靠性和性能的影响(如果适用)。

有关更多详细信息,请参阅OpenStack git 提交良好实践。另请参阅Erlang/OTP wiki 中的编写好的提交消息,了解有关提交消息规则的良好简短总结。应遵循的良好提交消息示例

代码质量标准

不要重复代码。遵循DRY 原则

正确性、简洁性和清晰性是第一位的。应避免不必要的聪明才智。如果必须聪明才智,请确保它被封装起来,周围有有用的注释,并提供了良好的书面测试。

使用有意义的标识符,不要编造缩写(使用常用的首字母缩写词,如 TTL 没问题)。使用名词表示类,使用动词表示方法。确保你的标识符易于区分。

必须删除未使用的代码。如果代码块未从任何地方调用,请不要将其保留并注释掉。否则,它会导致混淆,并且有人最终会使用它,到那时它可能已经过时且损坏。

代码必须是模块化的。如果单个方法中的行数过多或缩进过多,类中的方法过多,继承层次过多,代码将难以阅读。找到一种将其分解的方法。

代码必须是松散耦合的。如果不同模块的内部方面之间存在过多的依赖关系,更改一个模块会不断破坏另一个模块。找到一种将模块之间的交互减少到简单的明确定义的接口的方法。

检查代码中的任何错误是否会出现在带有堆栈跟踪的日志文件中。

坏的

      try:
          # some code that fails
      except:
          pass

良好

      try:
          # some code that fails
      except Exception as exc:
          # some code that processes exc 
          logger.error(traceback.format_exc())