
通常如何进行代码审查? 您发送合并请求,获取反馈,进行更正,发送修订以进行第二次审核,然后获得批准并进行合并。 听起来很简单,但实际上审核过程非常耗时。
想象一下,您有一个包含数百行更改的池请求。 审阅者应花费大量时间来完全阅读代码并了解建议的更改。 结果,从创建池请求到批准整个过程可能需要几天的时间-对于更改的审阅者和作者而言,这都不是一件令人愉快的事情。 最终,审稿人仍然会错过一些东西的机会很大。 否则检查可能过于肤浅,在最坏的情况下,池请求可能会立即被拒绝。
事实证明,池请求越大,检查它的好处越少。
如何避免这种情况? 如何使审核者更轻松,更易于理解池请求并优化整个过程?
我们正在翻译后端开发人员Sergey Zhuk的文章,内容涉及Skyeng移动应用程序团队的代码审查过程。
变更类别
假设您有一项任务-在项目中实现新功能。 您正在处理的池请求可能包含不同类别的更改。 当然,其中将包含一些新代码。 但是在工作过程中,您可能会注意到一些代码需要事先进行重构,以便有助于增加新功能。 或借助此新功能,您想消除的代码中出现了重复项。 否则,您突然发现了一个错误并想解决。 最终的池请求应该是什么样的?
首先,让我们看一下代码可以进行哪些类别的更改。
- 功能上的变化。
- 结构重构-更改类,接口,方法以及类之间的移动。
- 简单的重构-可以使用IDE执行(例如,提取变量/方法/常量,简化条件)。
- 重命名和移动类-重新组织名称空间。
- 删除未使用的(死)代码。
- 更正代码样式。
战略回顾
理解这些类别中的每一个类别都有不同的理解非常重要,并且在考虑它们时,审阅者应专注于不同的事物。 可以说,每类变更都涉及自己的审查策略。
- 功能变更:解决业务问题和系统设计。
- 结构重构:向后兼容和设计改进。
- 原始重构:提高了可读性。 这些更改主要可以使用IDE来完成(例如,提取变量/方法/常量等)。
- 重命名/移动类:名称空间结构是否得到改进?
- 删除未使用的代码:向后兼容。
- 更正代码样式:大多数情况下,合并池请求立即发生。
使用不同的方法来检查不同类别中的更改,因此,花费在检查它们上的时间和精力也有所不同。
功能上的变化。 这是最长的过程,因为它涉及更改域逻辑。 审阅者将查看问题是否已解决,并检查所提出的解决方案是否最合适或可以改进。
结构重构。 与功能更改相比,此过程所需的时间要少得多。 但是,对于应该如何组织代码,可能存在一些建议和分歧。
在99%的情况下检查其余类别时,将立即进行合并。
- 简单的重构。 代码变得更具可读性了吗? -Merzhim。
- 重命名/移动课程。 该类是否已移至更好的命名空间?-Merzh。
- 删除未使用的(死)代码是merzhim。
- 更正代码样式或格式-Merzh。 您的同事不应在代码检查期间检查此内容,这是linter的任务。
为什么要对变更进行分类?
我们已经讨论过,不同类别的更改将有不同的修订。 例如,我们根据业务需求验证功能更改,在结构重构中,我们检查向后兼容性。 而且,如果我们混合使用几种类别,那么审阅者将很难同时记住几种审阅策略。 而且,很有可能,审阅者将在池请求上花费更多的时间,而不是必要的,因此,他可能会错过一些东西。 此外,如果池请求包含各种更改,则在进行任何更正时,审阅者将不得不再次修改所有这些类别。 例如,您混合了结构重构和功能更改。 即使重构执行得很好,但是该功能的实现存在问题,那么在更正之后,审阅者将不得不从一开始就查看整个池请求。 也就是说,重新检查重构和功能更改。 因此,审阅者在池请求上花费了更多时间。 审阅者应该立即再次审阅整个代码,而不是立即通过重构考虑一个单独的池请求。
到底什么不值得混淆
重命名/删除类及其重构。 在这里,我们遇到了Git,它并不总是正确地理解这种变化。 我的意思是当许多行发生变化时发生大规模变化。 当您重构一个类然后将其移动到某个地方时,Git不会将其视为移动。 相反,Git将这些更改解释为删除一个类并创建另一个类。 这在代码审查期间引发了很多问题。 并询问该代码的作者为什么要编写此丑陋的代码,尽管实际上,该代码只是从一个地方移到了另一个地方,并做了微小的更改。
任何功能更改+任何重构。 我们已经在上面讨论了这种情况。 这使审阅者同时记住两种审阅策略。 即使重构工作做得很好,我们也无法将这些更改延迟到批准功能更改之前。
任何机械更改+人为进行的任何更改。 “机械更改”是指使用IDE或代码生成完成的任何格式化。 例如,我们应用新的代码样式并获得3000行更改。 并且,如果我们将这些更改与人员进行的任何功能更改或其他更改混合在一起,我们将迫使审阅者在精神上对这些更改和原因进行分类:这是计算机所做的更改-可以跳过,并且需要人员进行此更改结帐。 因此,审阅者花了很多额外的时间进行检查。
例子
这是一个带有方法功能的池请求,该功能会自动关闭与Memcached的客户端连接:

即使池请求很小并且仅包含一百行代码,仍然很难验证。 从提交中可以看到,它包含各种类别的更改:
- 功能(新代码),
- 重构(创建/移动类),
- 更正代码样式(删除多余的底座块)。
同时,新功能本身只有10行:

因此,审阅者应审阅整个代码并
- 验证重构是否正确。
- 检查新功能是否正确实现;
- 确定此更改是由IDE还是由人员自动生成的。
因此,这样的池请求很难检查。 要纠正这种情况,您可以将这些提交分为多个分支,并为每个分支创建一个请求池。
1.重构:类提取

只有两个文件。 审阅者应仅检查新设计。 如果一切都井井有条-Merzhim。
2.下一步也是重构,我们只需将两个类移至新的名称空间

这样的池请求很容易检查;可以立即实例化。
3.卸下多余的底座块

这里没什么有趣的。 Merzhim。
4.功能本身

现在,具有功能更改的池请求仅包含所需的代码。 因此,您的审阅者只能专注于此任务。 池请求很小,易于检查。
结论
经验法则:不要创建混合更改类别的巨大池请求。
池请求越大,审阅者就越难理解您提出的更改。 最有可能的是,包含数百行代码的巨大池请求将被拒绝。 而是将其分为逻辑上的小部分。 如果您的重构是有条理的,但是功能更改包含错误,则可以轻松阻止重构,因此您和您的审阅者可以专注于功能,而无需从头开始查看所有代码。
在发送池请求之前,请始终执行以下步骤:
- 优化您的代码以供阅读。 代码比书面更具可读性;
- 描述拟议的变更,以便为他们的理解提供必要的背景;
- 创建池请求之前,请务必检查您的代码。 并像执行其他人的代码一样进行操作。 有时,它有助于找到您错过的东西。 这将减少拒绝您的池请求和更正次数的可能性。
我的同事
Oleg Antonevich告诉我有关将更改分为几类的想法。
在编辑器中:Sergey写了很多有关编程和PHP的有趣的东西,有时我们翻译一些东西: 流视频服务器 , 呈现HTML文件 。 随时在本文的评论中问他问题-他会回答!
好吧,我们还提醒您,我们总是有很多有趣的空缺供开发人员使用!