10 ms·
First rule: trust your colleagues. If you think you’re a good developer, chances are your colleagues are good as well (otherwise they wouldn’t be hired in the s
by tkiolp4 6y ago
First rule: trust your colleagues. If you think you’re a good developer, chances are your colleagues are good as well (otherwise they wouldn’t be hired in the same company as you; they wouldn’t even be in your same team).
Second rule: don’t spend time in small details. This usually means: syntax, variable names, line length, test coverage. If you care enough about this, then install tools that verify this automatically when opening pull requests. Variable names: I know, this seems important, but bad variable names doesn’t necessarily mean bad code (keep reading).
Third rule: think long-term. This is, is the code under review easy to maintain? It’s not that subjective: how many files are being touched? All of them just for one feature? Smells like we need to refactor something else before working in that new feature. Are good abstractions present? This usually means you know a bit about the domain model and: no inheritance, dependency injection (I’m not talking about containers), and well defined interfaces.
Rule zero: no bugs (or at least no evident ones).
Regarding looking at the code at once or going commit by commit: none is a good way. Start examining the client code (or tests, sometimes) and go up to the core code (the one that has less dependencies). Usually the client code will tell you if the feature has a “good” architecture.