TE
科技回声
首页24小时热榜最新最佳问答展示工作
GitHubTwitter
首页

科技回声

基于 Next.js 构建的科技新闻平台,提供全球科技新闻和讨论内容。

GitHubTwitter

首页

首页最新最佳问答展示工作

资源链接

HackerNews API原版 HackerNewsNext.js

© 2025 科技回声. 版权所有。

The Code Review Pyramid

206 点作者 adrianomartins大约 3 年前

18 条评论

willcipriano大约 3 年前
The more experienced I get the more I want to work on teams with a higher degree of trust than ones like this. Don&#x27;t get me wrong I think code review can be valuable but on high preforming teams it works best on a as needed basis.<p>The way this has worked successfully for me in the past is as a new joiner to a team you submit for code review every time. My first couple of reviews tend to have a good bit of feedback as I settle in to the style of the team and often a new language or framework. After a while though I become a domain expert for the projects I am working on, and code reviews end up becoming a rubber stamp. At that point it&#x27;s just something slowing me down. I&#x27;d request review when I need it, or I&#x27;m making a big change that will effect everyone, otherwise there are many cases where there is really only one way to skin this cat and it&#x27;s totally perfunctory.<p>This sort of arrangement also requires a bit of ownership, if you see reports floating around about an endpoint you just worked on having issues you have to be able to be trusted to jump on it. I feel like trust and ownership are touchy feely people problems so tech people invented code reviews and linters to try to make up for not cultivating the sort of culture where they aren&#x27;t required on every single little thing.
评论 #30769515 未加载
评论 #30767952 未加载
评论 #30767985 未加载
评论 #30768257 未加载
评论 #30768164 未加载
评论 #30777439 未加载
评论 #30768261 未加载
评论 #30773502 未加载
评论 #30774773 未加载
评论 #30772293 未加载
评论 #30776449 未加载
owlbite大约 3 年前
Completely different to the order I use, which basically amounts to &quot;how hard is it to fix this later&quot;.<p>So most important questions are: (1) API must be as correct as possible, especially if its exposed outside of the immediate team. Changing APIs is a royal pain. (2) Documentation of what&#x27;s going on, especially for non-obvious stuff. If we come back to this in 3 years time and the author has moved on, do we have any hope of fixing it? (3) Testing. What degree of confidence do we have this is doing the right thing? Will it protect me when I change it in a few years time?<p>Any minor bugs or code style issues can be fixed later as&#x2F;when they are found and cause problems.
评论 #30769900 未加载
mannykannot大约 3 年前
While I recognize the problem as real and significant, and I think a hierarchy of concerns is a valid way to mitigate it, I feel there are a few problems in this particular pyramid.<p>Of all the issues that might be raised in a review, which are the most important to fix? I find it difficult to imagine a ranking in which incorrect functioning (including security vulnerabilities and truly inadequate performance) are not at the top (answering the ranking question with &quot;all of them&quot; would just be a way to avoid the issue.)<p>In this pyramid, issues of this nature are to be found at all levels except the top one, mixed in with more-or-less subjective ones, such as &quot;Is a new API generally useful and not overly specific?&quot; - pointless flame wars have erupted over issues such as this (also, orthogonally, code review is late in the game to be asking this one.)<p>For a review to be successful, its participants need to be able to restrain themselves from going down rabbit holes that could not lead to necessary rework - or a strong moderator, if the individual participants cannot act with adequate self-restraint.
评论 #30767225 未加载
评论 #30779464 未加载
评论 #30768174 未加载
AngeloR大约 3 年前
I&#x27;ve been thinking about code reviews a lot.. and I think a big problem with existing code reviews is that for most orgs, code reviews were a way to move planning to the end of the development process instead of keeping it up-front.<p>I think the foundation of the pyramid (API and Implementation semantics) is often discussed in code reviews when it should have almost no place. By the time you get to the code review not only should these have been ironed out and shared with the whole team. What you should be looking at is simply adherence to the plan, and calling out deviations.<p>I wrote about a small rat about this: <a href="https:&#x2F;&#x2F;xangelo.ca&#x2F;posts&#x2F;code-reviews-are-failure&#x2F;" rel="nofollow">https:&#x2F;&#x2F;xangelo.ca&#x2F;posts&#x2F;code-reviews-are-failure&#x2F;</a>
评论 #30770446 未加载
mpalczewski大约 3 年前
First thing I review is readability.<p>Code should be readable. Once code is readable it makes reviewing the rest much easier. Readable code is more maintainable and problems jump out at you.<p>Stuff other than readability is important, but if you focus on readability it makes the rest of the review go very smoothly.
评论 #30767463 未加载
评论 #30767211 未加载
评论 #30783448 未加载
评论 #30767103 未加载
simonbarker87大约 3 年前
I like the attempt at making this process smoother, I guess where to place importance of different aspects relative to each other is always down to opinion and interpretation.<p>I wrote this piece a while ago focussing on using the Must&#x2F;Should&#x2F;Could principle to speed up the review process and put a less opinionated framework in place to help keep code review moving.<p><a href="https:&#x2F;&#x2F;careerswitchtocoding.com&#x2F;blog&#x2F;moscow-the-best-code-review-technique-youre-not-using" rel="nofollow">https:&#x2F;&#x2F;careerswitchtocoding.com&#x2F;blog&#x2F;moscow-the-best-code-r...</a><p>It’s not an original thought from me but many people hadn’t heard about it and off the back of this post I’ve had a few people get in touch to say they’ve implemented it in their team and it’s helped improve code review speed.
sz4kerto大约 3 年前
Ours is different.<p>Tests (end-to-end and integration) are at the bottom. If there are no tests that prove that the code works, we won&#x27;t really look further because we don&#x27;t even know if the code implements the right thing. Then comes interfaces, then implementation, then style.
评论 #30769795 未加载
makecheck大约 3 年前
An important one often overlooked (due to our tendency to focus on just the “diff”): <i>what else should be changing</i> or <i>could be affected</i>?<p>Very easy to forget to change code that isn’t highlighted in the review.
BrissyCoder大约 3 年前
<a href="https:&#x2F;&#x2F;en.wikipedia.org&#x2F;wiki&#x2F;Law_of_triviality" rel="nofollow">https:&#x2F;&#x2F;en.wikipedia.org&#x2F;wiki&#x2F;Law_of_triviality</a><p>When I was on a larger team that actively participated in sprint demos was regularly a witness to this. Senior types that came along and needed to be heard - not really having a deep understanding of the application - would pick at trivial consistency issues in the UI or whatever. Such eye-rolling behaviour and hard to believe they didn&#x27;t realise how transparent it was. Probably have been guilty of it myself tbh.
smokey_circles大约 3 年前
One thing I wish I knew was coming was what my first code review as the tech lead was gonna be like. I was as unprepared as the grads code was.<p>Things I&#x27;ve learned about my own professional code preferences since<p>1) I will not sacrifice readability for any reason. That is the hill I&#x27;m willing to die on. Reducing code is sometimes hard and that&#x27;s fine that people like to challenge themselves by doing so, but not at the expense of readability. You will forget the clever trick you employed, I promise<p>2) Business objectives are second class to system stability. This is because the primary business objective is to be online. Failing a few transaction is bad. Failing all the transactions is much worse.<p>3) There is no such thing as self documenting code. There is just good nomenclature. Documents always improve someone&#x27;s experience. On boarding documents for new devs will save you so much trouble.<p>4) I&#x27;m a huge fan of in memory caches. I hate seeing them in code. When they work, they&#x27;re wonderful. But when there are issues, they&#x27;re closer to demons than to bugs.<p>5) Data Structures. Data Structures, Data Structures, Data Structures. And then more Data Structures. Easy pre-optimisation, and these days it usually just means knowing when not to use &quot;the default&quot; data structure (such as ArrayList in java)<p>7) One day (if you&#x27;re not there already), you will be on the otherside of this review. You&#x27;re almost guaranteed to remember the bad times, so pay extra special attention to when the process is working well.<p>A simple trick I&#x27;ve learned is to repeat a process that worked from a previous role at a new role who&#x27;s process is not working. Works more times than not.
评论 #30776462 未加载
VBprogrammer大约 3 年前
I&#x27;ve noticed a phenomenon where I sometimes have to comment about something near the pointy end of the code review pyramid in order to get my brain into the correct mindset to really start grocking how and why someone has made the changes they have. After I break this barrier it often leads to the deeper questions about whether it is doing what it supposed to be doing.
majormajor大约 3 年前
Across 6 jobs now I&#x27;ve never seen code reviews focus primarily on formatting or style over functionality or future-looking concerns. How common is this, really?<p>At the last couple places formatting has been handled by auto-linters; before that, it was hit or miss, but never a dominant topic of argument.
评论 #30773957 未加载
dhzhzjsbevs大约 3 年前
&gt; When it comes to code reviews, it’s a common phenomenon that there is much focus and long-winded discussions around mundane aspects like code formatting and style, whereas important aspects (does the code change do what it is supposed to do, is it performant, is it backwards-compatible for existing clients, and many others) tend to get less attention.<p>Code formatting and style discussions that don&#x27;t start and end with, hey, our code formatter is broken have absolutely no place in code review. Also, keep your damn nits to yourself. If it&#x27;s a nit, it&#x27;s not important.
ilovecaching大约 3 年前
This doesn&#x27;t seem correct to me. Each of these categories should bear equal weight, I won&#x27;t spend less time on code style or tests to spend more time on API design. My code review checklist is a checklist. I spend as much time as necessary on each item in the checklist, no more no less.<p>I also think that code style, fuzzing, and benchmark tests are the most important items on my checklist. It keeps the code base from becoming a jenga tower that is brittle to change. It also keeps us honest as to whether a feature is a perf hit or not. Tests are far more important than this graphic portrays them as.
评论 #30766845 未加载
neebz大约 3 年前
previous discussion: <a href="https:&#x2F;&#x2F;news.ycombinator.com&#x2F;item?id=30674159" rel="nofollow">https:&#x2F;&#x2F;news.ycombinator.com&#x2F;item?id=30674159</a>
deschutes大约 3 年前
I find reviews work best when you place a strong bias towards shipping workable if imperfect code. Quick reviews are also a strongly positive thing.<p>On the other end are fixed processes. Have as little of this as possible, automate it if you can. That means formatting, linters and shallow static analysis.<p>All of this is mainly in service of building trust. If you cant have some measure of success there you are truly doomed.
nhoughto大约 3 年前
I&#x27;ll use this I think, very handy. I&#x27;d probably emphasis all the text&#x2F;descriptions more too, lots of important nuggets in there that aren&#x27;t obvious when glancing, like automating the top portions of the pyramid, humans shouldn&#x27;t be mis-formatting code and others human comment on it anymore.
Quarrelsome大约 3 年前
aren&#x27;t we missing something at the base of the pyramid:<p>&gt; does it actually work?<p>I feel like the foundation of any code review has to be a bug analysis to ensure that the feature works, doesn&#x27;t create regressions or fail in another supported use-case.
评论 #30767246 未加载
评论 #30768357 未加载