Hacker News new | ask | show | jobs
by jsbg 331 days ago
> We found files shown earlier in a PR to receive more comments than files shown later

I often stop reviewing a PR if I have too many comments on it already that need to be addressed so I wouldn't say this is necessarily what it sounds like (later files get less attention). Also, a comment on an early file might address a concern in a later file that will be fixed by the time I actually review the later files.

That said, keep your PRs short and you won't have to worry about stuff like that.

2 comments

Read this as though it has a visibly bulging temple vein:

Does that mean you do the thing where I respond to your review, request a re-review, then get comments about code that was already there a week ago?!

If your PR had lots of stuff that needs comments then I don't think you can reasonably expect me to check it fully in the first pass. E.g. if your first 3 functions are indented wrong, I'm going to write that they're indented wrong and not bother reading the rest of your PR until you've fixed that.
> E.g. if your first 3 functions are indented wrong, I'm going to write that they're indented wrong and not bother reading the rest of your PR until you've fixed that.

Just add a linter and precommit hook, it’s not a good use of anyone’s time to go back and forth over trivialities.

I know, just using it as a clear example. Point is if your PR has a lot of low-level issues then I'm not going to start on the higher level review until you've fixed them.
Can't seem to get anyone else to Baird the train of small CRs. It's so obvious