TE
TechEcho
Home24h TopNewestBestAskShowJobs
GitHubTwitter
Home

TechEcho

A tech news platform built with Next.js, providing global tech news and discussions.

GitHubTwitter

Home

HomeNewestBestAskShowJobs

Resources

HackerNews APIOriginal HackerNewsNext.js

© 2025 TechEcho. All rights reserved.

No Code Reviews by Default

44 pointsby thomaspaulmannalmost 4 years ago

32 comments

jrumbutalmost 4 years ago
I&#x27;m happy this works for them but I find so much value, even in a two person early stage team, of having someone take even the quickest of glances at your code.<p>It&#x27;s a very direct form of communication and also stops a lot of chaos especially when you&#x27;re moving fast early on.<p>I can get behind a &quot;no nitpicking&quot; policy in some circumstances and always a &quot;be kind&quot; policy, but really someone should be looking over the push if only for their own edification.
评论 #27606943 未加载
评论 #27608705 未加载
评论 #27606935 未加载
kelnosalmost 4 years ago
The article makes a big deal about the idea that code reviews show a lack of trust, and that not having mandatory code reviews shows that people trust each other.<p>This just isn&#x27;t correct. People make mistakes all the time. It&#x27;s human. Code reviews can help catch some of them before they become bugs that need to be fixed. This has absolutely nothing to do with trust. If you have people who think that requiring code reviews mean that you don&#x27;t trust them, you&#x27;ve hired some overly-sensitive people who have inflated opinions of their abilities and are ignorant of basic realities about software development.<p>I agree that some reviews end up being perfunctory (either due to a busy reviewer, or the trivial nature of a change), but as much as I am generally a process hater, I think code reviews should be generally mandatory, but with a culture of merging small, seemingly-safe changes without review, when soliciting one would be an unnecessary burden. Even then, everyone working on a shared code base should be opening PRs for those sorts of things, even if they just merge them immediately without comment. That way you can use PRs as a communication tool (to let others know what&#x27;s going on; even more useful in a heavily-developed repository), and if someone really does want to look over something after the fact, they can do so.
palijeralmost 4 years ago
And the pendulum swings yet again. Eventually, with scale, I feel this approach will realize why the code reviews were implemented in the first place.<p>I love experimenting with new approaches and challenging old processes that everyone seems to do &quot;because that&#x27;s the way it&#x27;s always been&quot;, but I feel too often we don&#x27;t understand why a process like reviews are as popular as they are before we remove them. These processes grew organically, it wasn&#x27;t some manager somewhere who came up with the idea. It was a solution to a human problem, and the hardware hasn&#x27;t changed.<p>I find this is a great read if I&#x27;m not explaining myself clearly on my phone.<p><a href="https:&#x2F;&#x2F;fs.blog&#x2F;2020&#x2F;03&#x2F;chestertons-fence&#x2F;" rel="nofollow">https:&#x2F;&#x2F;fs.blog&#x2F;2020&#x2F;03&#x2F;chestertons-fence&#x2F;</a>
评论 #27613072 未加载
评论 #27607237 未加载
estebarbalmost 4 years ago
I&#x27;m not really convinced. Code reviews are a great opportunity for learning, improve and share knowledge. Are extremely useful for onboarding new members on a project. You can learn a lot seeing others code, asking questions in place or receiving suggestions.<p>Sadly a lot of teams and companies see that as a performance metric (how measuring patches&#x2F;commit measures how good somebody is programming?) or as a process that must be passed asap.
评论 #27607065 未加载
评论 #27607979 未加载
评论 #27606793 未加载
davedxalmost 4 years ago
As someone who worked on code bases with sometimes fairly big teams before the days of mandatory code reviews: I applaud this. I feel like the culture of code review has become way too serious and strict, to the point we need a “Nit:” syntax because yes, people take issue with the very biggest and very littlest parts of your contributions these days.<p>Everything has to be perfect. Everything has to be “clean”. It can really get quite stifling, especially when you remember a time before code review when working, high quality software was somehow still delivered.
评论 #27607088 未加载
评论 #27606997 未加载
评论 #27616231 未加载
评论 #27606798 未加载
评论 #27640733 未加载
评论 #27606898 未加载
Nursiealmost 4 years ago
I’m an experienced programmer and I hope a pretty good one. At least I’m paid that way so maybe it’s true.<p>I make mistakes. Sometimes there’s a typo. I sometimes miss that there was already a utility function that basically does that. Sometimes another engineer knows a better way to achieve the same thing. Sometimes I left in something unnecessary and missed it when I reviewed the changes.<p>I <i>like</i> having eyes on my code because it gives me reassurance and occasionally I learn things. It’s not an issue of trust.
评论 #27607351 未加载
trhoadalmost 4 years ago
Hell, I code review my _own_ code before anyone else does. The amount of issues I&#x27;ve spotted simply by viewing them as a PR in Github is insane. Code-blindness is a real issue!
评论 #27615262 未加载
throwaaskjdfhalmost 4 years ago
An approach that combined &quot;no code reviews&quot; with &quot;continuous deployment&quot; would be similar to coding on the production server, but with a better audit trail and consistent deployment across machines.
评论 #27606956 未加载
评论 #27607073 未加载
评论 #27606965 未加载
dekhnalmost 4 years ago
So... code review isn&#x27;t part of your annual security audit? Specifically, your SOX narrative doesn&#x27;t cite code review as a method used to prevent single-party attacks?<p>Huh.
评论 #27606634 未加载
评论 #27606652 未加载
评论 #27606630 未加载
throwaway4goodalmost 4 years ago
Looks good to me.
评论 #27606628 未加载
the_gipsyalmost 4 years ago
Seems out of question because companies are obsessed with minimizing risk. Instead, development crawls at snail pace because it takes way too long to integrate patches and too many developers get distracted with nitpicks. Or they just blindly approve, and PRs are just an extra timesink without any benefits. The developers hand off ownership at git push, everything after is out of control, bureaucracy , or entirely someone else’s job.
ManlyBreadalmost 4 years ago
I&#x27;ve worked in a place that didn&#x27;t do code reviews. The argument here was that you&#x27;re supposed to think about every case and ask questions even for what seems to be a trivial matter. That was the &quot;official&quot; explanation, the reality was that the boss was dumb enough to claim that &quot;we don&#x27;t have time and the client doesn&#x27;t pay us for code reviews&quot;. This has continously lead to more bugs and comments like &quot;what is this, why is this code here, who did it this way, it makes no sense&quot;. It also turned deployment into a nightmare since there always had to be at least two people on standby in case some piece of code starts wrecking havoc in production. I have managed to successfully convince the people working there to change the approach by forgetting a WHERE clausule in an SQL script which has caused havoc in the production system and forced several people to work at night and on the weekend. No one has caught it in testing because the change - despite the severity - was tested by a very inexperienced person and the testing environment has almost no data in the database so no one noticed the problem at this stage. I don&#x27;t feel bad at all about this because the change was big and I actually asked for review (even though we had no such process) and I was ignored. It also wasn&#x27;t the first time something like this was happening at that company, it was like that for years. The havoc could be even worse if I made a similar mistake somewhere else in the script, it would take weeks to clean up the mess and it would no doubt cause a huge financial loss for the client. There were some people who actually quit that company over the lack of such oversight and so did I. I think it&#x27;s an important failsafe mechanism, no less valuable than unit tests for example.
评论 #27616201 未加载
netikalmost 4 years ago
Yeah, no.<p>Even with a team of experienced engineers we find bugs and mistakes in PRs daily.<p>Having reviews encourages knowledge of the entire system and why things work the way they do, or, why something was designed a certain way.<p>For a two person write and sell beta level code as fast as possible startup maybe this approach is fine, but mostly it is not.<p>Go slow and make things work&#x2F;last.
评论 #27607488 未加载
评论 #27610762 未加载
z77dj3klalmost 4 years ago
Pitting code review against trust and responsibility is just disingenuous. The point of code review is not that you distrust others. It&#x27;s that everyone makes mistakes and often someone else having a quick look over your changes can come up with some really cheap wins that are safer, more maintainable, faster, etc.
coderddalmost 4 years ago
&gt; Pull requests don&#x27;t prevent bugs<p>Doesn&#x27;t match my experience. There&#x27;s two levels to this:<p>A good review can catch bugs directly. But even more important, it can dispute an obscure design, ask clarification for intent of ambiguous pieces of code, ask for added tests.<p>A hassle short term, but saves accumulated debt long term.
Stratoscopealmost 4 years ago
&gt; <i>Pull requests don&#x27;t prevent bugs.</i><p>This is my pet peeve with code reviews. They revolve around (1) only <i>reading</i> the code, not pulling the branch and testing it, and (2) reading <i>only the changes</i>.<p>One time I was about to give an approval on a PR because the Python code looked fine. Then I thought &quot;eh, maybe I should pull and test this branch, just in case.&quot;<p>With the new code in place, there was a marshmallow dataclass ValidationError in another part of the code that was not part of this change. This dataclass was used to ingest a JSON response from an API built by another team, and by default it had strict checking, including rejecting unknown JSON properties.<p>The developer had been testing against an older version of this API that didn&#x27;t have the new properties, and I was testing with a more recent version. The other team seemed to follow what I think of as the &quot;default JSON API contract&quot; which is that any changes or removal of existing properties need to be coordinated with the consumers of the API, but they felt free to <i>add</i> new properties to the JSON object. And this broke the strict marshmallow validation.<p>The dataclass had been added some time ago, but this problem never arose because we weren&#x27;t actually using this API until the new code in the PR started calling it.<p>The fix was simple enough; I just added this inside the dataclass:<p><pre><code> class Meta: &quot;&quot;&quot;Ignore unknown fields that we don&#x27;t use.&quot;&quot;&quot; unknown = marshmallow.EXCLUDE </code></pre> You might ask, &quot;Shouldn&#x27;t this problem have been caught by a unit test?&quot; But realistically, who is ever going to write a unit test that says &quot;let&#x27;s add some random new property to the mock JSON response and make sure it doesn&#x27;t raise an exception&quot;? Especially when there are some 50 API calls like this.<p>In the case of a JSON API response created by some loosely-coupled team, you&#x27;re better off not wasting the time on that. Instead, keep the strict checks and unit tests on properties you rely on, and use the marshmallow.EXCLUDE to ignore new ones that you don&#x27;t care about.<p>Whether you agree with that or not, this is the kind of bug that no one would ever catch in a &quot;read the code changes&quot; review.<p>It reminds me of the quote attributed to Donald Knuth: &quot;Beware of bugs in the above code; I have only proved it correct, not tried it.&quot;
taylodlalmost 4 years ago
Haven&#x27;t done required code reviews in over 15 years. No need to. I care more about your tests and how your tests impacted how you wrote your code. That tells me everything I need to know.
评论 #27607406 未加载
GGfpcalmost 4 years ago
The fact that people open PRs because they are working on a new part of the code means that there&#x27;s knowledge silos in the team that could be easily solved with code reviews.
评论 #27606790 未加载
aliasElialmost 4 years ago
I think that in general a good idea to have code reviews with new team members. It gives a good handle to talk about code organization, procedures and team conventions. You can also the new member to review modifications from other team members.<p>Code reviews become a necessity when you have really awful developers on your team. Fortunately I have not encountered many of them, but there was one developer that managed to make errors in even the simplest modifications.
sys_64738almost 4 years ago
Code reviews keep developers honest. Left to their own devices, developers will employ their terrible habits which sneak into the code base. Code reviews isn&#x27;t just about reviewing the code but it&#x27;s raising the bar for what is actually presented as complete. It separates sloppy code from complete, fully tested code. Developers are lazy and will look for shortcuts if standards are not maintained.
avelisalmost 4 years ago
My biggest sticking point is that the team write tests where applicable for every PR submitted. Is it fullproof? No. Does it ask for trust? Yes. YMMV
sitdownalmost 4 years ago
From the changelog:<p>* April 13, 2021 New command: Empty Trash<p>* April 28, 2021 Fixed an issue that would cause Empty Trash command to not work in some cases<p>Maybe you should have reviewed that code?
评论 #27608802 未加载
评论 #27607217 未加载
secondcomingalmost 4 years ago
I&#x27;d like to see their job titles, because if you don&#x27;t do reviews you don&#x27;t need to pay for senior engineers. Part of being a senior engineer is taking responsibility when broken code is released.<p>Also, I wouldn&#x27;t like to be working for that company and then get something like a PagerDuty alert when you&#x27;ve no idea what has been released.
评论 #27607838 未加载
jefftkalmost 4 years ago
You want process that is appropriate for your situation. I&#x27;ve worked in environments where code reviews and even tests did not make sense, primarily ones where development speed matters above everything else and the risk of something broken is low.<p>In most environments, however, code reviews are worth it for the mentorship alone.
GhostVIIalmost 4 years ago
I think this pretty clearly wouldn&#x27;t scale very well unless you have incredibly good hiring practices, and complete testing coverage. But it is interesting that it works for them at their scale.
eithedalmost 4 years ago
I&#x27;d want somebody else to look at my code - the more I spend on my code the more blinded I become to the issues I&#x27;m trying to fix while sacrificing others.
pixel_tracingalmost 4 years ago
Future: * Breaking News: Raycast shuts down, a spokesperson from the Raycast team had this to say; “It was.... oh god.... oh no! If we only reviewed that code!”
eCaalmost 4 years ago
It seems slightly off to argue no code review = trust, since that also argues code review = lack of trust.<p>(I don’t argue against no code reviews (though I lean in favour).)
pcmoneyalmost 4 years ago
From my understanding code reviews are one of the few rituals in modern software development that has been empirically shown to provide benefits.<p>It seems the proposers of this might have had a dysfunctional or even toxic culture. Code reviews should be educational, safe, and fairly blameless. Also breaks knowledge silos.<p>Engineers that don’t like their code to be reviewed are a people smell.
hulitualmost 4 years ago
Looks like Microsoft or Google. We wrote the code, the user shall test it.
theptipalmost 4 years ago
&gt; Pull requests discourage trust. Proposing every code change that somebody else has to approve doesn&#x27;t feel encouraging...<p>I can&#x27;t get behind this one. Sure, you can spin it this way, and maybe if you were in a traumatic environment with jerks as reviewers you might think this is a universalizable statement. But in a good team with good communications, pull requests are a good way of getting folks on the same page<p>&gt; Pull requests don&#x27;t prevent bugs.<p>Objectively false. (And trivially falsifiable; I&#x27;d bet every reader here has either found a bug in someone else&#x27;s PR or had a reviewer find a bug in their PR in the last month. I know I have, on both counts.) They don&#x27;t prevent _all_ bugs, but they do prevent some bugs. Lots of research on this, the general push for &quot;shift left&quot; is based on empirical findings that it costs less to fix a bug the earlier in your process you catch it.<p>If the author meant &quot;don&#x27;t prevent all complex bugs&quot;, then sure, but I feel that&#x27;s an unreasonable demand for any process; you don&#x27;t have to catch all bugs in order to provide a positive ROI.<p>&gt; We rely on fast iterations with user feedback.<p>If your business model is not hurt by shipping bugs to users, then the value of fixing bugs before you ship is low. More mature products&#x2F;companies don&#x27;t really have this option.<p>&gt; Pull requests slow down. Code reviews aren&#x27;t high priority for engineers. They have to make time to clean up their review queue. In reality, PRs are an afterthought and not what you want to spend your time on as an engineer.<p>I think this is the actual problem. Many orgs have a code review process that makes it painful for developers to iterate quickly on their ideas. You can avoid this problem by not doing code reviews, or you can fix this problem by doing code reviews better. The latter is hard though! If you make &quot;PR response time&quot; a metric for your team, create a slackbot to nag on outstanding PRs, and generally as an engineering leader keep the focus on fast reviews as a priority, then you can make this a cultural strong-point instead of being a source of drag.<p>&quot;Accelerate&quot; by Forsgren et. al. talks a lot about 1) the value prop of, and 2) some techniques for, speeding up the cycle time from commit-to-production and idea-to-production. This is not easy, but I think in most cases (i.e. for most company&#x2F;team sizes), dropping code reviews is throwing out the baby with the bathwater.<p>More generally, I think this post is falling into the (very common) trap of giving uni-directional advice; &quot;reviews slow us down, therefore they are bad&quot;, rather than more nuanced analysis like &quot;for our stage where quality is far less valuable than iteration speed, we think it&#x27;s not a good investment of time and effort to build a strong mandatory reviews process&quot;. This is a fine position to take, though it&#x27;s less snappy and I suspect won&#x27;t get you onto the front page of HN. But you don&#x27;t need to make claims about how code reviews damage trust or don&#x27;t actually provide value to justify this tradeoff.<p>And if you&#x27;re honest about why you&#x27;re making the tradeoff, you&#x27;ll be better placed to change the decision when the tradeoff falls in the other direction; a company that builds into its DNA the idea that &quot;Code reviews harm trust&quot; is going to avoid mandatory code reviews well past the point that they would provide positive ROI.
lrvickalmost 4 years ago
Without code review a compromised workstation can push malicious code that will make it to production and compromise all user data with little chance of being noticed.<p>This is incredibly irresponsible.<p>Never skip code review if for no other reason than security.