Pull Request 审查指南
对 Kibana 的每一项更改都必须达到高标准，虽然 pull request 的质量责任最终在于作者，但 Kibana 团队成员作为审查者有责任在审查过程中进行验证。
坦率地说，不可能构建一个具体的要求列表来涵盖审查 pull request 时可能遇到的所有情况，因此本文档试图列出一些常见的显而易见的要求，同时也概述了我们在进行任何 PR 审查时应持有的一般理念。
不希望也不打算让 PR 审查采取本文档的形式，其中最顶层的指南始终首先应用，而最底层的指南始终最后应用。整个文档应该被视为一个整体，而不是必须按顺序完成的一系列待办事项。
虽然审查过程始终由 Elastic 员工完成，但这些指南适用于所有 pull request，无论它们是由社区成员还是 Elastic 员工编写。
本文档的目标受众是 pull request 审查者。对于 Kibana 维护者来说，PR 审查是我们在贡献过程中唯一可以完全控制的部分。任何给定 pull request 的作者可能不了解我们对 pull request 的最新期望，他们可能从未阅读过我们的指南。作为审查者，我们有责任指导大家完成这个过程，但如果没有一套通用的、有文档记录的原则，很难始终如一地做到这一点。
Pull request 作者也可以从阅读本文档中受益，因为它有助于在作者和审查者之间尽早建立一套共同的期望。
每个 pull request 都是不同的，在审查任何给定的 PR 之前，审查者应考虑进行 PR 审查的最佳方式，以便如果最终拒绝更改，则尽早拒绝。
例如，对于包含新 UI 功能的 PR，审查者可能希望尽早进行产品层面审查。另一方面，也许作者正在提交过去由于关键架构决策而被拒绝的新功能，在这种情况下，审查者可能应该专注于架构的合理性，然后再深入研究其他内容。
我们希望在所有 pull request 中遵循许多离散的要求和指南，但有三件事特别重要。
我们是否希望产品中包含此更改？
仅仅因为一个人花费时间构建了一些东西，并不意味着我们希望该更改存在于产品中，这就是为什么我们鼓励每个人在开始工作之前打开 issues 来描述他们想要做什么。
我们添加的每个功能都会永久性地占用我们构建和支持其他功能的精力。我们实际上对可以维护的功能数量有一个有限的容量，因为它们都需要我们花费一些时间和精力，因此，不仅要审查项目增强功能的质量，还要审查它们增加的价值、增加的维护负担以及它们与项目长期目标的一致程度。
情况也会随着时间的推移而改变，因此持续重新评估我们围绕是否希望项目进行某些更改所做的决策非常重要。有些更改需要花费大量时间来构建，并且有可能在我们完成它们时，情况已经发生了足够的变化，以至于我们不再希望项目进行更改。
此更改在架构上是否合理？
围绕更改的技术架构做出错误的决策意味着我们立即承担了技术债务，这将不必要地分散我们未来改进项目的精力，因为我们需要维护现有的更改、追踪错误并在未来最终重新架构该区域。
没有一种万能的方法来评估给定的更改在技术上是否合理，因此我们必须依靠我们构建软件的具体知识和经验。当两位经验丰富的开发人员对什么是架构上合理的事物持有不同意见时，这尤其困难，因此要求作者和审查者都表现出同情心，尽一切努力互相理解，并就前进的道路达成共识。
这里的目标不是找到一种最佳架构，因为这不太可能被限定。通常有很多架构上合理的解决方案可以解决任何问题，因此审查者应该客观地专注于该目标，而不是他们自己对更改应如何架构的愿景是否“更好”。
是否有足够的测试来确保此更改将来不会中断？
如果我们依靠手动测试来捕获错误，我们将无法扩展我们的开发工作并继续有效地改进和维护产品。手动测试是我们最后一道防线，也是确保产品稳定性的最无效且最昂贵的方式。这意味着我们对产品所做的每一项更改都需要有足够的测试覆盖率。
这不仅仅是足够多的测试文件的问题。测试中的代码本身与产品中的代码同样重要。
我们所有的代码都应该有单元测试来验证其行为，不仅包括“快乐路径”，还包括边缘情况、错误处理等。当您更改模块的现有 API 时，应该始终至少有一个失败的单元测试，这反过来意味着我们需要验证所有使用该 API 的代码是否在必要时正确处理更改。对于足够高级别的模块，这意味着我们在产品中存在重大更改，我们需要相应地处理。
除了广泛的单元测试覆盖率之外，PR 还应包括相关的功能和集成测试。在某些情况下，我们可能只是测试一个与文件系统、网络、Elasticsearch 等集成的编程接口（例如，服务）。在其他情况下，我们将通过 HTTP 测试 REST API 或将屏幕截图/快照与先前已知的可接受状态进行比较。在最坏的情况下，我们正在使用 selenium 在运行中的 Kibana 实例上进行基于浏览器的功能测试。
增强功能几乎总是以广泛的单元测试作为基础，以及功能和集成测试。错误修复应始终包括回归测试，以确保相同的错误将来不会再次出现。
审查者不仅仅是评估代码本身，他们还在评估产品中面向用户的更改的质量。这通常意味着他们需要在本地检出分支并“试用”它。除了“我们是否希望产品中包含此更改”的细节之外，审查者还应寻找错误并评估该功能在实施后的可访问性和有用性。应特别注意错误场景和边缘情况，以确保它们在产品中得到很好的处理。
拥有相对一致的代码库是我们构建可持续项目的重要组成部分。由于任何给定时间都有数十名活跃的贡献者，我们依靠自动化来帮助确保一致性 - 我们通过 CI 强制执行一套全面的 linting 规则。我们还在推出 prettier，使其更加自动化。
对于无法轻松自动化的事情，我们维护了一个 风格指南，作者应该遵守，审查者在审查 pull request 时应该牢记。
除此之外，我们就进入了主观领域。像“这不是很可读”这样的说法几乎没有帮助，因为它们无法被限定，但这并不意味着审查者应该完全忽略由于编写方式而难以理解的代码。没有一种明确的“最佳”方式来编写任何特定的代码，因此追求这种方式不应该是我们的目标。相反，审查者和作者都必须接受，可能有许多不同的适当方式可以通过代码完成同一件事，只要贡献利用了这些方式中的一种，我们就处于良好的状态。
另一方面，如果审查者指出某个特定的代码块难以理解，那么第一个查看代码的人很难理解它。如果审查者强烈认为这是一个问题，他们应该提出这个问题供作者考虑，并可能提出替代方案。审查者必须务实，他们的目标应该是推动讨论向前发展，而不是简单地设置障碍。
如有疑问，依靠代码库中的“现有技术”，尤其是在正在贡献的代码库的区域及其周围，通常是一个好主意。
有时，一个人可能会受到特定贡献的启发，从而引入一种新的代码风格，而我们已经针对这种风格制定了不同的风格指南或“现有技术”。可以在 pull request 中提出这一点，但最终该讨论应该分支成一个单独的 issue 或 pull request，以更新适当的指南。如果此更改是由审查者提出的，则不应阻止原始 PR。如果更改是由作者提出的，那么他们可以选择更新 PR 以与我们现有的指南保持一致（首选），或者他们可以选择完全阻止 PR 进行单独的风格指南讨论。
吹毛求疵是指评审者在 Pull Request 中挑出琐碎和不重要的细节，并要求作者进行更改。这是一个完全主观的类别，不可能进行普遍定义，并且制定一个让每个人都满意的关于吹毛求疵的总体政策同样不切实际。
评审者应该可以随意给出他们对 Pull Request 的任何反馈，无论它多么琐碎。作者也应该可以随意忽略他们认为琐碎和无关紧要的反馈。
通常，评审者对于他们将要给出的反馈是否是吹毛求疵有自己的看法。虽然不是强制性的，但将反馈标记为“小意见”可能会很有帮助，例如“小意见：这里添加一个换行符可能会有所帮助”。这有助于作者理解您的意图。
评审者和作者之间发生意见冲突是很常见的，有时很难调和这些意见。理想情况下，大家可以本着这些指导原则的精神共同努力达成共识，但如果行不通，最好引入第三方进行讨论。我们的 Pull Request 通常有两个评审者，因此合适的第三方可能已经很明显。否则，请联系最合适的职能部门或技术领导，如果某个领域不明显。
一项反馈是否适合 Pull Request，通常取决于给出反馈的动机。
要求作者主要基于“我会如何编写这段代码？”的心态进行更改是不合适的。评审者没有编写代码，他们在评审过程中的关键目的不是将贡献塑造成仅仅是他们自己编写代码的形式。如果评审者想要提供这种类型的反馈，他们应该按照吹毛求疵部分中提到的那样将其限定为“小意见”，以明确作者可以选择接受或忽略它。
带有煽动性的反馈，例如“这太烂了”，根本不是反馈。它既刻薄又无益，而且永远不合适。
为所有可能的 Pull Request 建立一个全面的检查清单是不切实际的，但这并不意味着我们缺乏一组可以枚举的具体最低要求。以下项目应针对任何 Pull Request 进行双重检查
CLA 检查通过
CI 作业运行并通过
遵守我们各种风格指南的精神
具有全面的单元测试覆盖率
自动化测试提供高度的信心，确保更改在没有手动验证的情况下继续工作
包含适当的产品文档 (asciidocs)
任何新的 UI 更改都对不同能力的人员无障碍，包括但不限于颜色方面足够的对比度、键盘导航和 aria 标签
在适当的情况下，应采取足够的安全保护措施，尤其是在渲染 API 响应、解析表达式、编译 URL、加载外部内容或重定向时
PR 标题总结了更改内容（没有“修复 bug 编号 123”）
PR 描述包括
- 更改内容的详细摘要
- 更改的动机
- 如果 UI 发生变化，则提供屏幕截图
- 指向由 PR 关闭的每个问题的链接（例如，Closes #123）