Can you explain your code review process? Maybe we're just doing it wrong, or maybe we're just talking about different types of changes. We typically do it at the final merge, after developer and unit testing, before QA, and most of my code is new systems, new subsystems or substantial features.
Someone sends me something to review and I see a notification pop up with a summary, regardless of what I'm doing at the time. More than 90% of the time (maybe more like 98%), it is from one of my team members, who has sent out something I knew they were working on because we had chatted (in a meeting, or at our desks, or on IM) about the need for it, and often also the approach they're planning on taking.
A lot of times (maybe around 50%?) this is configuration, documentation, or a very small code or test change. These I immediately skim for obvious problems like typos or confusing phrasing, check a diff of the config for those changes to make sure it does what it thinks it does, and almost always immediately approve the change with no comments. These often take less than 5 minutes.
Maybe 10% of the time, it's a fairly small functionality-preserving refactor. I defer these until I have some free time, because they aren't critical. I review them with an eye toward making sure I think they actually preserve functionality and decrease complexity and/or confusion. These can take 15 minutes to an hour, but aren't very disruptive because I can do them when I'm not otherwise busy.
Maybe 30-40% of the time, these are medium to large functionality changes to the part of the codebase I am intimately familiar with (again: the vast majority of these are coming from members of my team). Depending on what I'm doing at the time, I'll either start reviewing it immediately, or take a note reminding myself to take a look when I get a chance. I actually start by spending some time reading the description and thinking about whether I think what it's claiming to do is even the right thing to do; if not, I comment on that before even reading the code.
After that, I dig into the code. I start by reading the whole thing at a pretty high level to see if it seems to do what the description says, focusing on method names and signatures and especially on documentation.
If all looks well at the high level, I like to start going through the changes "outside-in", that is, starting with the higher-level methods and digging into the lower-level ones over time. Mostly what I'm looking for is whether everything makes sense to me when I read it, if not I ask for renaming, restructuring, or just more documentation if it seems fundamentally complex or confusing. In other words, I try to read it from the perspective of someone coming in cold and trying to figure out what's going on. This will be the case for everyone reading this code in the future, including me and the author in a few months. I also try to think hard about what assumptions are being made, whether they are right, and what should be done about it if I'm unsure.
After all this, I look at the tests. I start by checking for good coverage. If some things appear uncovered, I ask for tests, or suggest ways it could be tested if it seems tricky. If the coverage is good, I'm honestly kind of lazy about reading the tests themselves, looking for obvious ugliness, but mostly assuming that if they're passing and covering the branches, they're probably fine. Sometimes I suggest integration tests that I think might be useful.
If the change is in code that has lots of downstream clients, I'll usually do another pass focused on whether we might be breaking them somehow. Sometimes I have to do research on libraries that are being used or could be used instead of something else.
This can all take 15 minutes to maybe two or three hours of work time, sometimes spread across a few days of wall time, including any back and forth. I wouldn't say I go "line-by-line", though I do think I end up reading all the (non-test) lines at least once. I do feel I should completely understand what's going on, but that's usually built into my experience with the code in question.
It does sound like you may be involved with more brand new systems than I usually am. My feeling about those is that it's helpful for an author to make their changes in very small pieces, so that it is as obvious as possible to a reviewer. I also think the description of the change is very important in those cases, so that the reviewer understands the purpose and approach of the new functionality. If I were to guess from your brief description, I would say it is possible you are reviewing things in larger-than-ideal chunks.
In general, though, it is definitely fairly expensive to do code reviews, but I think it improves quality and maintainability to an extent that makes it worthwhile.
The code review I do is subtly different in those two contexts. Within my team, my goals are to keep us to a very high standard of committed code, it should be clear, simple, and well documented. We want our output to be relatively reusable, well tested and testable, and well designed. This goes beyond our newly written code, we are stewards of libraries and tools, and we should maintain them as well as we can.
On the other hand when reviewing google-wide code, I am, yes, looking for bugs, but mostly looking for good "mastery" of python. That is, I'm explicitly looking for idiomatic python from people who are not as experienced with it. These reviews also are often larger and longer.
Within my team, we have a number of practices that help make code reviews fast:
- Most changes are small: within my team I reviewed 3 changes of more than 250 lines this quarter, compared to a dozen or so of <10 lines
- Most of the code written is really good. I rarely, if ever, have to mention lint errors, nor do I often come across bugs.
- My comments mostly revolve around idiomatic ways of solving problems and, as a team python expert, "pythonic" approaches, and better approaches using the python standard library and known tooling (ie. "You're reinventing the wheel here, just use XYZ", or "this is confusing, use pathlib.path instead.")
- The other big thing I comment about is clarity and readability, as a reviewer, I want to be able to follow your code, code is written more than it is read, and so most of my comments are things that amount to "This is difficult to follow as someone reading, can you refactor this to make it easier to read." These comments vary in both necessity and type, they can range from "hey maybe reorder your conditionals here and see if it reads better" to "holy wow it took me 10 minutes with a repl open and asking for help to figure out what the heck this function is doing, this needs to be refactored."
- And the last thing I suggest is future work to be done: "We should refactor this method in the future, we're having to work around it a lot", or a very common one: "We can parameterize these tests".
- The other big thing we do to make code reviews quick is that everyone does them. They don't just fall on the TL or manager or whatever. I review code for everyone, and everyone reviews my code, and as a result the code is not mine but the team's. Nitpicks aren't really a thing, because if someone is picky just to be picky, they'll get the same treatment in turn (and also because that's a dick move and we don't do that).
Note that for the most part, the things I'm looking for are pretty easy to notice, unidiomatic code is obvious, and "this is difficult or tricky" is also very clear. I still notice bugs and such, but they're rare, and we write code to make reviewing easy and errors obvious, and as a result have fewer of them.
The across-google reviews are on average bigger than within my team, and often result in more comments, because the writers are less good at python (everyone on my team has python readability, whereas the people I'm reviewing for python do not, otherwise they wouldn't need my review). They take longer. Many of them are still relatively small, but yeah the big ones that cross 500 or 1000 lines, and are in an unfamiliar part of the codebase, those do take a significant chunk of time to review (normally ~1-2 hours), but they're rare, I get one of those every couple of weeks.
In general I'd say the answer to how to make code reviews go faster is
1. Encourage small changes when possible
2. Use code review to teach best practices
3. Encourage writing code for readability and testability
4. Have everyone review code
Edit: Added a fourth thing
A style guide and linter also remove the arguments about minutia. Google's python style guide is a lot less strict than the go or java styles, for example, and even still most of the style comments I make when reviewing code amount to "be internally consistent" about something, and those are nits.
And that whole "reviews take 5 minutes" doesn't mean that they aren't useful. I often find something that is either confusing, or a typo, or dumb mistake/overlooked thing or a non-idiomatic way of doing something, and leave a comment expressing that. The review still takes 5 minutes, but I've caught a bug or a bit of technical debt later.
As a code-writer, I have more or less the same opinion. I would flatly refuse to work somewhere that doesn't do code review. It makes working in the codebase difficult, and makes personal development more difficult.