Horizon/Reviews

Horizon 代码审查

审查标准

补丁应从四个方面进行审查：代码、测试、文档和部署。

代码

• 代码是否正确？格式是否良好，是否符合以下链接中的代码风格指南？
• 它是否有效？
• 它是否解决一个问题，还是多个问题？每个补丁应只解决一个问题。在进行 CSS 清理或实现更大的功能时，这可能会变得有些模糊。本意是限制将两个不相关的更改合并到一个补丁中。这会使更改跟踪变得非常困难，如果需要将更改回滚或 cherry-pick 到稳定分支，我们将回滚多个更改，而不是仅仅回滚有问题的一个。
• 正在审查的代码与单个更改相关。如果您发现与更改周围的代码相关的问题，但这些问题不相关，则这不是有效的审查意见。请在 launchpad 中提交错误报告，以便单独解决该问题。
• 此更改是否有益？为了迎合个人偏好而进行的过度代码修改总体上是有害的。这种更改比任何事情都更具破坏性。
• 此更改是否正在重新实现已有的功能？我们不需要/不想要相同功能的 3 种实现。
• 如果补丁包含对 python-*client 的新/更改的调用，该功能是否受该客户端的最低支持版本的支持，请参见 openstack/requirements。如果不是，是否包含功能/扩展检查？
• 提交消息是否引用了正在解决的 bug-id 或蓝图？所有更改都应在 launchpad 中跟踪。代码是否实际解决了链接的蓝图或错误？如果涉及蓝图，则对批准的计划和 UI 原型的任何偏差应进行说明。
• 如果您不确定更改的主题，请要求 Horizon 的功能专家进行审查，或询问其他核心成员。欢迎在审查中提问。
• 所有用户可见的字符串必须可翻译。

测试

• 如果如何测试补丁不清楚，应在提交消息中或作为审查的评论中包含说明。如果测试方法不清楚，请礼貌地要求作者添加说明，并在添加后进行审查。
• 补丁是否有测试？所有补丁都应通过单元测试进行验证。这可以防止未来的回归。包含测试对作者和项目都有益。代码重组、清理或移动可能会被豁免。但所有测试都应通过。
• 在提交更改之前运行测试。始终测试您的更改并获得即时反馈，而不是等待 CI 将该信息传递给您，这总是一个好主意。
• 集成测试不应增加失败次数。

文档

• 更改是否已记录？更改/添加设置的补丁应在 horizon 文档 中记录。更改底层小部件工作方式的补丁也应在 horizon/doc 以及代码中记录。旨在供重用的新功能，例如小部件，应进行文档记录。

部署

• 考虑此更改是否会影响已部署的 Horizon 版本，并设想部署者需要执行哪些升级路径来吸收这些更改。如果需要迁移，是否已记录？
• Horizon 的大部分内容都由部署者覆盖。此补丁是否出于纯粹的装饰目的而更改了 CSS 类、python 类、模板路径、HTML 元素、JavaScript 方法等的名称。如果是这样，应避免这些更改。如果存在充分的理由进行更改，则应将其包含在提交消息中。

参考文献

与 Horizon 相关的代码审查主题（包括 django-openstack-auth、tuskar-ui）。

请添加对审查有用的良好参考资料。

• OpenStack 代码审查指南
• OpenStack 编码风格指南
• i18n
• Javascript 指南（正在进行中）
• Git 提交消息
• 关于提交到稳定分支（backports）

审查链接

• 直接链接
  • Horizon
  • django-openstack-auth
  • tuskar-ui

• Horizon + django-openstack-auth 审查仪表板
  • Gerrit 仪表板创建器很有用。
  • 仪表板 #1
    • 由 https://github.com/stackforge/gerrit-dash-creator/tree/master/dashboards 生成
  • 另一个审查仪表板（涵盖 Horizon、django-openstack-auth 和 tuskar-ui）
    • 由以下仪表板定义生成

 [dashboard]
 title = Horizon Review Inbox
 description = Review Inbox
 foreach = (project:openstack/horizon OR project:openstack/django_openstack_auth OR project:openstack/tuskar-ui) status:open NOT owner:self NOT label:Workflow<=-1 label:Verified>=1,jenkins NOT label:Code-Review>=0,self
 
 [section "Needs Feedback (Changes older than 5 days that have not been reviewed by anyone)"]
 query = NOT label:Code-Review<=2 age:5d
 
 [section "Your are a reviewer, but haven't voted in the current revision"]
 query = NOT label:Code-Review<=2,self reviewer:self
 
 [section "Needs final +2"]
 query = label:Code-Review>=2 limit:50
 
 [section "Passed Jenkins, No Negative Feedback"]
 query = NOT label:Code-Review>=2 NOT label:Code-Review<=-1 limit:50
 
 [section "Wayward Changes (Changes with no code review in the last 2days)"]
 query = NOT label:Code-Review<=2 age:2d