Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

Warm take?

Just reject and ask for an explanation OR pair with the coder to have them explain the code.

If the reviewer can’t understand it without an explanation the rest of the team can’t understand it unless they git blame then ask the coder, assuming they are still at the company!

The “dump 1000 lines of previously unseen and undiscussed code as a code review at a reviewer” method is an antipattern.

Either the reviewer should be heavily involved or break it into smaller, well explained chunks with design documentation to point to.

This saves everyones time in the long run.



As a senior developer of 30+ years experience, I agree and was going to post the same thing.

If code is difficult to understand, I consider that to be a bug. And if, for some reason, the section of code actually is difficult to implement, I would at least want a pretty good comment explaining the context around it and the reasons for the approach taken.

Not only will this help improve the code, but if it is a senior developer, it will help them to understand where they've become blind to a particular complexity. It is great feedback to have someone explain that they don't understand code that seems 'fine' to you.


In principal I agree, and it's not like I'm losing entire days to this sort of thing (well, once I did, but only once), but there's a tricky interplay of experience + lingering imposter syndrome. I do a fair bit of jumping around between languages and frameworks, so when I, say, come back to JS after being away for 6-9 months and I see something unfamiliar, there's usually a 60/40 split that it's a bug vs something the JS community has decided is a new "industry best practice". Before immediately going to the author asking for an explanation, I attempt to do my due diligence and be sure that I'm on the 60% side of that split.

It's just that lately the split has been more like 60/30/10 between bug, some new thing, and garbage AI spew.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: