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