3 ms·
300 LOC in 10 minutes. Or 2 sec per loc. Or for average 30 char line, 600wpm reading speed. OK. There is little you can review properly in 10 minutes unless y
by nenenejej 1y ago
300 LOC in 10 minutes. Or 2 sec per loc. Or for average 30 char line, 600wpm reading speed.
OK.
There is little you can review properly in 10 minutes unless you were already pairing on it. You might have time to look for really bad production-breaking red flags maybe.
Remember the underlying reasons for PR. Balance between get shit done and operational, quality and tech debt concerns. Depending on what your team needs you can choose anything from no review at all to 3x time reviewing than coding. What is right depends on your situation. PR is a tool.
- rustybolt 1y agoAgreed, 300 lines will take me a lot more than 10 minutes to review properly! Depends on the specific changes of course, but generally speaking.
- whstl 1y agoAlso depends on the codebase. 300 lines is nothing in some boilerplate-heavy codebases I've worked at. After seeing the same patterns for the hundredth time, it's easy to detect deviations. Not to mention linters and typing helps a lot too. Not a fan of those but oh well.
- enlyth 1y agoI like to actually checkout the branch I'm reviewing and run the code myself to observe if it does what is claimed, that usually takes up at least 10 minutes in itself, sometimes more. From my experience most of the issues I find are actually from this type of observation rather than actually reading the code and imagining what it does in my head.
- qiine 1y agothe mind remains a poor compiler ;p
- franktankbank 1y agoYour linter/tests are for catching real errors. Review is to understand the shape of it mostly IMO. I could probably fairly easily review 300 loc if its not a particularly confused shape.
- tcoff91 1y agoIt all depends on the lines. It can take a long time to review a 1 line change to a critical function that is used everywhere in the app and it can take minutes to review 1000 lines of declarative UI code.