What are you strict about when reviewing code? What do you always point out in your peer reviews?
For further actions, you may consider blocking this person and/or reporting abuse
What are you strict about when reviewing code? What do you always point out in your peer reviews?
For further actions, you may consider blocking this person and/or reporting abuse
Pranav Bakare -
Hamza Khan -
akhil mittal -
riddhi -
Top comments (5)
FP
I think I'm biased but, after learning about functional programming 2 years ago, I'm always looking for ways to make code more functional.
So whenever I see functions like this...
... I would point out how functions should have the least amount of side-effects and find a cleaner way to achieve this outside of the function.
Comments
I can't disagree more with the statement "good code should document itself". I do find value in a comment given I'm not the only one reading this code. If there's a routine that takes my brain 3 minutes to understand, I'd ask OP to put a comment on why this piece of code exists.
Error handling
This is a very common pitfall I see on PRs. Devs accounting for success paths only. Read the DB and return the value; cool, but how are you handling errors? create object and route to /dashboard; cool, but what happens if the object can't be created?
Code structure & common patterns used
Is the code structured correctly and following the same trends and patterns as the rest of the application?
If it breaks from those patterns is it a good reason to do so?
Code quality
Does it follow good namig conventions?
Are FP principles used?
Are unit or integration tests are present?
If existing code is modified and it had automated tests to cover it, have the tests been updated to also check the new logic?
Maintainability
If i think certain parts are badly written or too complex, unreadable, or could be simplified I might suggest alternatives.
Note to self when reviewing code
Can you automate more to make some general mistakes occur less often?
Great learnings from here. I like how you strive for codebase consistency (is this similar to what we have elsewhere) and also for automation (avoid style discussions by introducing a common linter). Thanks for sharing!
Readability
I don't really care how many one liners you have, if they're not readable, they aren't useful. Inline comments, correctly named variables, and DRY code is my number one thing.
Lint it please
I'm a little OCD about this, but I hate when other programmers don't lint their code :( Makes me really annoyed.
Top Level Documentation
Each file should have some JSdoc quality explanations of the file, I should be able to quickly read through a summary to see what each method and class does.
All in for ripping off one-liners here too!