Genuine question, what was the most comments you've left in a single code review?

Just reviewed pull request submitted by a developer working for a contractor company and needed to leave 70 comments. Seventy.

Opened LinkedIn and saw a post from that same developer saying he left the contracting company an hour ago. I still can't believe it.

Good for you. That's more than I ever left on a single PR. I hope there were a lot of changes...

40 to 50 comments on a PR which spanned 50+ files

Did he leave because you left that many number of comments? Or he worked on this change after he decided to leave?

What kind of comments were those? Something that should be dealt with by a linter? Or was the PR just too big?

I quit after 20 comments or so and will just do a second review after that's fixed. Imagine reviewing all your requested changes again at once. Imagine reviewing 70 changes. That's some serious work and hard to do without mistakes

makes sense

seen that before. someone does a shit job cuz they know they don't care and gonna leave. ofc they won't tell anyone and you're left treating them like they're retarded and they're more work than they should be because of their "attitude". sigh

needs context.

70 comments on 3 lines of code is catastrophic, 70 comments on 3000 lines is okay-ish.

I mean a lot of comments is okay if justified. But the comments "you suck" and "wtf is this shit?" are not really necessary.

@Demolishun but WTF/min is the only reliable measurement of code quality.

@electrineer I guess its the difference between slander and libel.

If 1 on 1 reviews count , about 2h

Finally got some energy to give some more context here.

It was a pull request providing integration with 3rd party provider - Docusign to a Symfony (PHP) project.

That being said, there were some 30 files added, but it's something that shouldn't be a rocket science for a senior developer with 7-8 years of experience. Add client from sdk, couple entities, messages plus some intialization command and thats it.

Taken from my summary comment in that pull request:

"From what I’ve found:
- Pretty sure the code is not runnable - some parts will throw runtime / logic errors
- Basic oop were not respected - encapsulation, missing type declarations, single responsibility, declaring Doctrine fields as public
- Naming conventions are to say lightly, weird in several places - not respecting any PSR or best practices
- PSR-12 formatting was not respected - a lot of blank lines and whitespaces, though this can all be fixed with PHP-CS-Fixer
- There is a lot of typos, shorthand names and unnecessary comments

This code review turned out in almost a full man day of analyzing, finding and explaining how to fix basic mistakes which should not even be present in a pull request as any half-experienced developer should know how to take care of those himself.

All in all, this looks more like a draft or work in progress and not a production ready code to be shipped to a client"

@asgs the company that provided him knew he will quit and tried to push his half-done work in last minute to us final product ready for review.

@electrineer funny enough, linter not being run was the smallest issue here, if I documented that also, would easy go over 100 comments..

@jestdotty that was from a summary comment which I wrote *after* I found on LinkedIn the guy is leaving their company and they tried to pull a dick move on us. It was supposed to be little mean (though I would call it no unfiltered bullshit), as it is for their CTO to read it.

Original 70 comments were all really nice and understanding, 90% of them had phrases such as "please do this", "sorry but this won't work" and "i see your way but lets do it like that" in them accompanied by nice and patient explanations.