评论和批准拉取请求
Commenting and Approving Pull Requests

原始链接: https://www.jakeworth.com/posts/on-commenting-and-approving-pull-requests/

这种代码审查方法优先考虑团队信任和快速进展。作者即使在存在非阻塞性评论(例如吹毛求疵、建议或问题)的情况下也会批准拉取请求(PR),认为留下评论表明了参与度并提供了学习机会。 关键在于*信任*团队在认为有价值时考虑和实施反馈。这在快速的CI/CD流水线和能够处理琐碎问题的工具(如linter和格式化程序)的配合下效果最佳。作者使用“常规评论”(例如“nitpick:”或“suggestion:”标签)在批准时明确评论意图。 然而,这种方法依赖于一个PR批准系统,该系统*不会*在新提交时自动重置。对于重大阻塞性问题,作者会评论并可能阻止PR。最终目标是培养一个高度一致的团队,大多数反馈都是非阻塞性的,从而实现自信的“评论并批准”工作流程,优先考虑指导和发布代码。

这次黑客新闻的讨论集中在软件开发中,对拉取请求(PR)进行评论和批准的最佳实践。核心思想是简化审查流程,特别是针对经验丰富的工程师。 用户建议利用“带建议的批准”等功能(如Azure DevOps中找到的),并将代码审查的重点放在*想法*的清晰度上,而不是严格的把关,尤其是在已经有健全测试的情况下。 一个关键的收获是认识到代码审查的*成本*——速度至关重要。虽然策略可能规定24小时的审查窗口,但实际的*典型*审查时间(目标为2-3小时)会显著影响团队速度。讨论强调通过直接对话和PR提醒等工具来解决瓶颈,以确保及时审查,并避免因时区问题导致的延误。
相关文章

原文

After reviewing a lot of pull requests, I’ve settled on a simple default: if my comments are all nitpicks, suggestions, questions, or non-blocking issues, I leave them and approve the PR at the same time.

Here’s some detail on how it works. But first, two clarifying questions.

Why leave comments if I’m approving?

Comments show that someone has thought about the problem and the solution, and cares about what’s happening to the code. Occasionally, they offer a chance to learn or to surface misunderstandings, assumptions, or hidden risks.

I almost always leave a comment on each PR I review, even just observations: “This class is getting big, we might want to consider adding a presenter,” or praise: “Thanks for cleaning this up!”

Why approve, if I’ve left comments that I think are worth implementing?

Because I trust my team. I know that my comments will be considered, and if they’re useful, implemented.

My team is fast enough to make the change. CI runs fast enough to validate it. The time it takes to do it isn’t an obstacle.

Process Points

Here are a couple of process points that this workflow assumes.

First, you have to trust your team. If you don’t trust them to read and think about your comments— if they aren’t a strong enough signal— work on that.

Also, some repositories are configured to reset approvals when a new commit is pushed to the PR branch. If that’s your configuration, this approach is less effective because your review is removed when a change implementing your suggestion, or anyone else’s, is committed. There’s still value there because your approval is documented on the PR, but it’s not ideal.

Additionally, some repos can be configured to automatically merge PRs when all requirements are met, one of which might be your approval. Consult your repo configuration before taking action.

This process also works better with low-configuration tooling that negates a lot of trivial nitpick comments. With linters, auto-formatters, type checkers, security scanners, etc. running in development and CI, some lower-value comments don’t have to be written.

Finally, all the comments I write are contextualized by Conventional Comments. I use labels like nitpick:, question:, suggestion:, and issue (non-blocking): to clarify intent as I approve.

Here are a few other tools, like Conventional Comments, that have been recommended to me:

For more major feedback, the kind that should block work, I might comment only, or comment and block.

It’s something I decide case-by-case, and it’s different for different kinds of issues, projects, and the person whose code I’m reviewing.

Code reviews are an expensive place to find a blocking issue or design flaw. On the kind of teams I’ve worked on the most, startups and SMBs, this frequently points to an upstream misalignment. Other teams might have stricter code-review gates, and it’s more common

You want to get to a place where most feedback is non-blocking because the team is highly aligned. And then you can comment and approve with confidence.

Challenge: Comment and Approve!

For the skeptical, my suggestion is to try this with someone on your team with whom you have rapport. You can say in a comment on the PR: “This looks good, approving. I think that the rename is worth doing, but otherwise good to go.” Most engineers I know will consider this advice and do it. Conversation over, and code probably better.

I want my code reviews to coach and teach, and this practice helps. When it errs, it errs on the side of progress rather than process.

It’s my way of saying: “I care about this change, and here’s how I think it could be even better. It’s your call when to ship it.”

联系我们 contact @ memedata.com