131–140 of 144 posts
One way I've forced others to review my code is to put early code up in a PR for feedback. It allows others to see the process. It also drives the point home that we are all human and don't magically poop out great code.
I am a junior developer and my latest feedback was that one of the main skills I should develop is to make better, more well-thought, critic and deep code reviews (including of PRs from more senior developers). Any tips on how to improve this? Would a checklist help? Have a clear process on what to review first?
A checklist can help. (he says, and then does a mental one because there isn't one nearby). What I look for is: 1) What's the problem being solved? Does this look like a reasonable approach? Is the code pythonic (Obv: for python)? 2) What edge cases are there? Does this handle the important ones? Does it punt properly on the less important ones? 3) Look for a short list of bug classes that have come up in the project…
I am a junior developer and my latest feedback was that one of the main skills I should develop is to make better, more well-thought, critic and deep code reviews (including of PRs from more senior developers). Any tips on how to improve this? Would a checklist help? Have a clear process on what to review first?
Also, read a lot of code reviews. Just like reading a lot of code is helpful for becoming a better developer, reading a lot of reviews is helpful for becoming a better reviewer.
My code is well commented. //eslint-disable-line all over the place.
Invoking eslint-disable needs to have comment of it's own justifying why the line in question is being skipped. What use is the linter if we just disable it everytime it complains?
I refuse to remove 'extrenious' parenthesis that make the code more readable to junior devs who may not know the language specific order of evaluation in a logical expression.
Something the author doesn't bring up, but that we started doing about 9 months ago at my company, is synchronous reviews. Meaning the committer is on the phone or in person with the reviewer. It's great. We don't do it for all PR's, but anything medium sized or above, or even small one's if they involve critical logic. The way we usually do it is the committer walks through the changes with the reviewer. Often the c…
> We have repositories for the PHP backend, our database schemas, our iOS (Swift/Obj-C) and Android (Java/Kotlin) mobile apps, infrastructure projects written in Go, C/C++, Lua, Ruby, Perl, and many other projects written in Scala, Node.js, Python, and more Why do organizations allow this? I realize that some platforms require their own languages (iOS, Android), but outside of that, just pick one or two and hold the…
Earlier quoted context omitted.
> Each language has special strengths and weaknesses, there is no silver bullet that excels at everything. Go, C/C++, Lua, Ruby, Perl, Scala, Node.js, Python... each of these are THE best choice for certain classes of problems (and terrible for others). Not quite true... not all languages have a sweet spot in a production environment. Node.js isn't particularly excellent at anything, the attraction is mainly "I know…
Ruby, Python, Perl, PHP, and (maybe) Lua aren't amazing at handling async operations like Node.JS can. Go is, in many respects, a great replacement for Python and Node in that it's simple and handles concurrency well. Scala and other JVM languages fall here too, but come with their assoc complexity. Python, Node, and Ruby have tons of battle-tested code which often makes the language the "strongest" if for nothing el…
BEAM languages are the sweet spot for concurrency right now.
Earlier quoted context omitted.
> Each language has special strengths and weaknesses, there is no silver bullet that excels at everything. Go, C/C++, Lua, Ruby, Perl, Scala, Node.js, Python... each of these are THE best choice for certain classes of problems (and terrible for others). Not quite true... not all languages have a sweet spot in a production environment. Node.js isn't particularly excellent at anything, the attraction is mainly "I know…
> Node.js isn't particularly excellent at anything That's not very kind. Yes it has its flaws but it's not bad at being a higher abstraction over async-everything, augmented with a huge module repository. Support for isomorphic javascript can come in handy for some applications. I've never done Scala but my understanding of it is that it could be a good fit for modelling certain types of business logic while being ab…
My point about Scala is that there doesn't seem to be a clear cut niche that it excels at. There are numerous other non-Java languages that run on the JVM that could be in the running, and they all get to take advantages of the whole Java ecosystem too.
Earlier quoted context omitted.
A checklist can help. (he says, and then does a mental one because there isn't one nearby). What I look for is: 1) What's the problem being solved? Does this look like a reasonable approach? Is the code pythonic (Obv: for python)? 2) What edge cases are there? Does this handle the important ones? Does it punt properly on the less important ones? 3) Look for a short list of bug classes that have come up in the project…
1000 should really be handled by automated tools. Takes useless burden from the reviewer, and emotionally easier for both sides too.
What I see as one of 5 maintainers of an open source project is that when a review comes back with a bunch of formatting comments, it's because the reviewer didn't step back and see the other parts. Raymond Hettinger has a talk where he discusses it, but it's the sort of feedback that can be given in almost any case and can obscure the more fundamental issues with the code.