git拉取请求_我在审查拉取请求时问的三个问题

git拉取请求

它可读(又称“美丽”)吗？ (Is it readable (a.k.a. “beautiful”)?)

Is the easy to follow? Is the code commented when necessary and self-documenting otherwise? Is there a way to write this code such that it is easier to read?

容易遵循吗？ 必要时是否对代码进行注释，否则进行自我记录？ 有没有办法编写此代码，使其更易于阅读？

As an example, variables and method names should be as short as possible while still being intuitive. For example, I will use a variable name like unprocessedOrdersHash despite its length because it is more readable than uOrdersHash.

例如，变量和方法名称应尽可能短，同时还要直观。 例如，我将使用一个变量名等unprocessedOrdersHash尽管它的长度，因为它比更具有可读性uOrdersHash 。

I am one of those developers who compares writing code with painting a picture, and I believe that it is always worthwhile to take the extra time to refactor code not necessarily to be more readable or more performant, but more beautiful. For example, I have spent probably too much time rearranging properties of an object in a mapper method such that they are in alphabetical order. It does nothing for performance, and nothing for readability of the code (as far as what the code is doing), but it makes the code look better and I tend to take more pride in my code when it is beautiful!

我是将编写代码与绘制图片进行比较的那些开发人员之一，并且我相信花额外的时间来重构代码不一定总是更易读或更高性能，而是更美观 ，这总是值得的。 例如，我花费了太多时间在映射器方法中重新排列对象的属性，以使它们按字母顺序排列。 它什么都不做了性能，并没有为代码的可读性(至于什么代码的作用)，但它使代码更好看 ，我倾向于采取更多的自豪感在我的代码时，它是美丽的！

是表演吗？ (Is it performant?)

Is the code performant and does it accomplish the intended result efficiently?

代码是否有效，是否有效地达到了预期的结果？

A common example of code that might not be performant is the famous “foreach within a foreach” (as an aside, at least one other dev has always taken pause when I checked in a foreach within a foreach, but the code has always gone to production in the end- it’s good to question something like a foreach within a foreach, but in my experience it truly is the best solution sometimes, especially in cases where the collection sizes of each foreach are and will always be very small).

一个常见的可能无法执行的代码示例是著名的“ foreach中的foreach”(顺便说一句，当我在foreach中检入foreach时，至少另一个开发人员总是会暂停一下，但是代码总是最后，最好在一个foreach中质疑一个foreach之类的东西，但是根据我的经验，有时候这确实是最好的解决方案，尤其是在每个foreach的集合大小总是很小的情况下。

是否一致？ (Is it consistent?)

“Consistency” refers to consistency with respect to code formatting as well as conformity to best-practices.

“一致性”是指代码格式方面的一致性以及最佳实践的一致性。

This includes:

这包括：

• Where do the curly braces go (new line or same line)?
  花括号在哪里(新行或同一行)？

• Are new variables declared with var or their explicit type?

  新变量是用var还是它们的显式类型声明的？

• How does the code interface with the database (e.g. ORM vs native library)?
  代码如何与数据库交互(例如，ORM与本机库)？

At some dev shops, the developers are very well-trained such that I never actually have to think about whether the code is consistent and follows established convention. For example, everyone knew to use var instead of the explicit type at one job. I had never seen a case at this company where someone, even an intern, checked in code using the explicit type in lieu of var.

在一些开发商店中，开发人员训练有素，因此我实际上不必考虑代码是否一致和是否遵循既定惯例。 例如， 每个人都知道在一项工作中使用var而不是显式类型。 我从未在这家公司看到过这样的案例：有人，甚至是实习生，都使用显式类型代替var来检入代码。

At other dev shops, you will need to pay attention to whether convention is being followed, but I suspect that the devs can be “trained” to the point where you no longer have to consider whether the code is consistent. All dev shops should strive to only have to worry about the first two- readability and performance.

在其他开发人员商店，您将需要注意是否遵守约定，但是我怀疑可以对开发人员进行“培训”，以至于您不必再考虑代码是否一致。 所有开发人员商店都应努力只担心前两个方面的可读性和性能。

拉取礼节 (Pull Request Etiquette)

All developers review pull requests differently. All have different criteria for when to push the “Approve” button and when to “Reject” a pull request. Often the “PR Protocol”- when to approve, when to reject, whether to reject, what’s worth commenting over or nitpicking- is set by the culture of the dev team. Many think that “Rejecting” a pull request is too harsh and choose instead to comment and simply not approve it until the comments have been addressed- this phenomena could be its own article.

所有开发人员对拉动请求的审查方式都不同。 对于何时按下“批准”按钮和何时“拒绝”拉取请求，所有标准都有不同的标准。 通常，“ PR协议”(何时批准，何时拒绝，是否拒绝，值得评论或挑剔的东西)是由开发团队的文化决定的。 许多人认为，“拒绝”拉要求过于苛刻，而是选择注释，根本就没有批准，直到评论一直的方式送这种现象可能是它自己的文章。

There may also be varying expectations of whether all issues should be caught by the code reviewer, or if PRs should be reviewed briefly and approved without thorough and complete review of the changes. Some of this comes down to the individual developer- not everyone loves to stop what they’re working on to read and review someone else’s code- but it starts with the “culture” of the dev shop.

对于是否应由代码审阅者来解决所有问题，或者是否应该对PR进行简短的审核和批准，而又不对更改进行彻底而完整的审核，可能存在不同的期望。 其中一部分取决于个人开发人员-并非每个人都喜欢停止他们正在阅读和查看他人的代码的工作-但这始于开发人员商店的“文化”。

我如何处理拉取请求 (How I Approach Pull Requests)

Personally, I like to take my time with pull requests. I consider it a privilege rather than a mundane burden (I don’t like to spend the full 8 hours per day writing code).

就个人而言，我喜欢花时间处理请求。 我认为这是一种特权，而不是平凡的负担(我不喜欢每天花整整8个小时编写代码)。

I will often pull down the feature branch to see and play with the code in my preferred IDE. It takes a lot longer than giving the code a quick glance (really only good for screening for obvious issues) but I see value in it. Usually when I review a PR, there’s a reason to Google (or “StackOverflow”) something I don’t know the answer to. Occasionally this even leads to me creating a new question on StackOverflow. I have learned about some very specific nuances of the C# language and its Roslyn compiler this way. What’s more, the longer you spend looking at a PR, the more likely you are to find a not-so-obvious issue.

我经常会下拉功能分支，以查看并使用我喜欢的IDE中的代码。 它比快速浏览代码花了很多时间(确实只适合筛选明显的问题)，但我认为其中有价值。 通常，当我查看PR时，有一个原因让Google(或“ StackOverflow”)知道我不知道答案的原因。 有时，这甚至导致我在StackOverflow上创建一个新问题。 我已经通过这种方式了解了C＃语言及其Roslyn编译器的一些非常细微的差别。 而且，您花在PR上的时间越长，找到一个不太明显的问题的可能性就越大。

I would compare reviewing a PR with viewing art. When some view art, not only are they spending several minutes looking at a single piece- they are also thinking about what they would change. As developers, we are lucky in that we can pull down a copy of the Mona Lisa and experiment with it.

我可以将评论PR与观看艺术进行比较。 当某些人观看艺术作品时，他们不仅要花几分钟时间看一件作品，而且还在思考他们将会改变什么。 作为开发人员，我们很幸运，我们可以提取一份《蒙娜丽莎》并进行试验。

奖金| 查看其他人的PR时不该做什么： (BONUS | What NOT to do when reviewing someone else’s PR:)

不要Nitpick (Don’t Nitpick)

Nitpicking someone’s code is a great way to demoralize them. Consider that we all have our own coding styles and what we consider to be the “best” solution to a problem. Things like an extra space between code blocks, and variable names that are different from what you would have chosen may annoy you, and may make the code base “imperfect”, but that will always be the case in a shared code base. In cases like these, where performance and readability aren’t impacted, and convention is being more or less followed, I find it best to leave it alone.

剔除某人的代码是使他们沮丧的好方法。 考虑到我们都有自己的编码风格以及我们认为是问题的“最佳”解决方案。 诸如代码块之间的多余空间以及与您选择的变量名不同的变量之类的事情可能会惹恼您，并使代码库“不完美”，但是在共享代码库中总是如此。 在这样的情况下，性能和可读性没有受到影响，或多或少地遵循了惯例，我发现最好不要理会它。

If these small “imperfections” are a really big deal to you, try leading by example instead. Create pull requests where you fix imperfections and put the offending devs on the PR as reviewers. It doesn’t have to be the exact code they checked in, at least not at first. Most developers will get the hint.

如果这些小“瑕疵”对您来说真的很重要，请尝试以身作则。 在修复缺陷的地方创建拉取请求，然后将有问题的开发人员放到PR上作为审阅者。 不一定是他们签入的确切代码，至少起初不是。 大多数开发人员都会得到提示。

不要传授您无用的知识 (Don’t Impart Your Useless Knowledge)

This is an interesting one. Don’t impart your useless knowledge. Here’s an example of what I mean. You read about C#’s new “range” operator while browsing Reddit the night before. You see an opportunity to use the range operator in a peer’s code you’re reviewing. You make a comment on the PR that they should use the new range operator (because they can!). Even though you approve the PR whether they implement the range operator or not, I am against this.

这是一个有趣的。 不要传授您无用的知识。 这是我的意思的例子。 您在前一天晚上浏览Reddit时了解了C＃的新“范围”运算符。 您会发现有机会在您正在查看的对等方代码中使用范围运算符。 您对PR进行评论，他们应该使用新的范围运算符(因为他们可以！)。 即使您批准PR无论他们是否实现范围运算符，我都反对。

To be clear, PRs are a great place to teach others while pointing out issues with their code. This includes domain knowledge, best practices, and company-specific protocol and convention. PRs are not the place to tell someone how they can rewrite their code in a way that is just as readable and just as performant. Aside from this, you would be introducing code that presumably most of the dev team would be unfamiliar with when they encounter it in the wild.

需要明确的是，PR是在教别人指出其代码问题时的好地方。 这包括领域知识，最佳实践以及特定于公司的协议和约定。 PR 并不是告诉别人如何以可读性和性能相同的方式重写代码的地方。 除此之外，您还将介绍一些代码，这些代码大概是大多数开发团队在野外遇到它们时都不熟悉的。

Save it for the Lunch-n-learns!

将其保存为午餐n学习课程！

不要盲目批准 (Don’t Blindly Approve)

We all do it. It’s high noon, you’re famished and you’re ready to go to lunch. A coworker stops you- “James, can you approve this PR real quick????” (it’s always real quick). You dash back to your computer, punch in the secret code that is very similar to the secret code you used to punch in before they made you change it, and there it is, the biggest, fattest PR that ever was. As you begin to frantically flip through the diffs, your stomach reminds you it’s 12:03 and the lunch gang is making for the elevators. Fuck it. Approve.

我们都做。 现在是正午，您很疲惫，已经准备好午餐了。 一位同事阻止了您-“詹姆斯，您能很快批准这个PR吗？？？” (这总是很快速的 )。 您冲回计算机，输入与您在更改密码之前曾经输入过的密码非常相似的密码，这是有史以来最大 ， 最繁琐的 PR。 当您开始疯狂地翻动差异时，您的肚子会提醒您现在是12:03，午餐帮派正在乘电梯。 他妈的 批准 。

It’s risky, but sometimes it works out just fine. That said, more often than not, there is a follow-up PR waiting for me when I get back from lunch. Some obvious, stupid bug was missed (wow that’s amazing, it slipped by two, three, four developers!), the build broke, and a fix needs to go out pronto! Your belly is full, and you have all the time in the world to review this PR- except you don’t, because it broke the build and everyone is affected. Here’s hoping there isn’t a bug in this second PR.

这很冒险，但有时效果很好。 就是说，当我从午餐回来时，经常会有后续的PR等待着我。 遗漏了一些显而易见的愚蠢错误(哇，太神奇了，它被两个，三个，四个开发人员滑倒了！)，构建失败了，需要立即进行修复！ 您的肚子饱了，您在世界各地都无休止地审查此PR-，除非您不这样做，因为它破坏了身材，每个人都受到了影响。 希望第二次PR中没有错误。

If it’s a matter of legitimately not having enough time to review others’ PRs, something needs to change with the way the shop develops software.

如果合法地没有足够的时间来审查他人的PR，那么商店的软件开发方式就需要有所改变。

翻译自: https://medium.com/dev-genius/the-three-questions-i-ask-when-reviewing-a-pull-request-8791b142e6cd

git拉取请求