Earlier quoted context omitted.
But, code formatting is not a solved problem, right ? You either have to fiddle until the formater outputs proper layout, or let it rest as is and never learn how to format your code, letting the code base leaking tons of probable less than ideal layout.
The proper layout is whatever the formatter outputs. That's the point. Everyone uses the formatter and accepts whatever it does, and nobody needs to think about it anymore.
The Code Review Pyramid
101–110 of 117 posts
Re: The Code Review Pyramid
#102Earlier quoted context omitted.
But, code formatting is not a solved problem, right ? You either have to fiddle until the formater outputs proper layout, or let it rest as is and never learn how to format your code, letting the code base leaking tons of probable less than ideal layout.
I configure Prettier once on my JS codebases and then never touch it again. Seems about as "solved" as is reasonably possible.
Re: The Code Review Pyramid
#103Earlier quoted context omitted.
But, code formatting is not a solved problem, right ? You either have to fiddle until the formater outputs proper layout, or let it rest as is and never learn how to format your code, letting the code base leaking tons of probable less than ideal layout.
I'll take a consistent less than ideal layout over whatever $HUMAN comes up with. But to me an inconsistent layout is the less than ideal one. And humans excel at inconsistency.
Re: The Code Review Pyramid
#104The more experienced I get the more I want to work on teams with a higher degree of trust than ones like this. Don't get me wrong I think code review can be valuable but on high preforming teams it works best on a as needed basis. 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…
This is scary to read. We use code reviews for changes and we’ve found that our more junior team members can bring a point of view our more senior team members can’t and vice versa. It’s one of the best parts of working in code for me.
With that said, code reviews only work, in my experience, if team members are strong communicators. Understanding each other, empathizing with issues, and being collaborative rather than tearing people down is critical.
Code reviews are a “we” activity not a “your” and “my” activity.
Re: The Code Review Pyramid
#105While 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. 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…
I think you are getting too into the weeds with these complaints. This hierarchy is pretty good. Performance and security are important, but they aren't always a full stop, where a change the breaks the API should always be a full stop. Each higher level is less likely to be a hard stop, and the border between levels is necessarily somewhat grey.
Re: The Code Review Pyramid
#106The more experienced I get the more I want to work on teams with a higher degree of trust than ones like this. Don't get me wrong I think code review can be valuable but on high preforming teams it works best on a as needed basis. 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…
The popularity this sentiment on HN terrifies me. Please don't do this. I keep finding rubber stamp code reviews (or none at all) while diving into codebases for where bugs were introduced, and 9 times out of 10, it's someone who didn't bother to get proper reviews for their code because "it's just something slowing me down". I can assure you that you have bugs and usability issues in your code, not just "linter issu…
Been on both camps of this argument. Now tend to favor that responsible seniors should be allowed a fast lane. It depends very much on your team dynamics. Seniors can be very cowboy so some of them actually need even more review than others.
Have been in some teams with very high junior/senior ratio where a junior would need a whole day to review a small thing a senior implemented in an hour, often because understanding the context leads to hundred rabbit holes of more new information, not the small diff itself. Add the one week that PR was waiting for someone to even notice it needed review and you realize the frustration. Vice versa a junior would take a week to implement something a senior can review in 15mins.
Do the math and you realize the senior will quickly get blocked.
Slowing down and teaching others in that situation is a as you say the right thing to do. But that also saturates after a while. You can only pair program x number of hours per day before getting exhausted, especially for a beginner this can feel like a lot of pressure to ingest so much knowledge at that pace. They need to slow down and get in their own zone as well. In the meantime, backlog is on fire with tasks that need to be released yesterday.
Hire smarter people? Yes please, would love to do that, where do I call? “Smarter” doesn’t always help either, the reality is that you often end up with domain experts within some areas of the product, even if you try to keep everyone on board.
Re: The Code Review Pyramid
#107Earlier quoted context omitted.
The popularity this sentiment on HN terrifies me. Please don't do this. I keep finding rubber stamp code reviews (or none at all) while diving into codebases for where bugs were introduced, and 9 times out of 10, it's someone who didn't bother to get proper reviews for their code because "it's just something slowing me down". I can assure you that you have bugs and usability issues in your code, not just "linter issu…
I disagree. If you have a practice that isn't effective (e.g. code reviews that aren't preventing bugs), doubling-down is almost never the correct solution. Code reviews are like unit tests; they're helpful under the right conditions. Mostly on the tricky bits or when a dev is working in an unfamiliar area. It's a common fallacy for folks to believe that if a little of something is good, then a lot of it must be bett…
Every place I've worked has required reviews on pull requests. Are there any notable tech companies that don't?
My colleagues sometimes find non-trivial errors in my code, and I find errors in theirs. Maybe we're just not as smart as the people you work with.
Re: The Code Review Pyramid
#108First thing I review is readability. 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. Stuff other than readability is important, but if you focus on readability it makes the rest of the review go very smoothly.
This is where I have found review to be really tricky. I have reviewed so much unreadable code that I lose faith sometimes. More to the point, readability issues are often perceived as nitpicks. Eventually after a few rounds of readability issues being addressed I can tell what the actual code is doing. At that point the major suggestions and some rework comes out, but now the author feels burned, nitpicked, and now instead of being done "their perfectly functional" code needs more work and the whole thing is not done. Do this a few times and relationships begin to break down.
Hence, I now try really hard to ignore readability until the end.
I don't know if this partly just hostile developers, different understanding of code. Ie, is code for humans to read, or for machines to execute? Or if it's dealing with feature factory developers that just want to keep adding despite how brittle the code is becoming. I presume the problem is somewhere in between, but to another extent the normalcy of the absurd where it is normal to spend 5 minutes per line of code to understand it, it is normal to havk in features and do manual regression testing, etc..
Re: The Code Review Pyramid
#109The more experienced I get the more I want to work on teams with a higher degree of trust than ones like this. Don't get me wrong I think code review can be valuable but on high preforming teams it works best on a as needed basis. 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 trust the people whose code I review to be competent developers. But everyone occasionally makes typos, misreads requirements, has blind spots, etc. Even if the creator is the expert and the reviewer is there to catch only obvious mistakes, that still has value.
> But everyone occasionally makes typos, misreads requirements, has blind spots, etc.
Case in point: OP used the word "effect" when he should have used "affect".
Just like pudding, there's always room for a code review.
Re: The Code Review Pyramid
#110Earlier quoted context omitted.
No, I mean like the primary function of every PR is to ensure the code does what its trying to do and doesn't (for example) fail to compile, or break the build or create an infinite loop or an out of memory exception.
On a dayjob closed-team project, if my coworkers are putting up PRs without making sure they meet functional requirements, that is a problem that needs to be solved several steps before the PR. If it's a junior person who doesn't know better, more working side-by-side with them to get to the PR. If it's a senior person dealing with a shitty spec, more discussion with PMs/designers before we start coding. If it's a se…
[deleted]