Comment by n4r9

Agreed. There's a whole checklist of what to look out for:

- Does it functionally achieve what it sets out to (as per tacker issue or PR description)?

- Does it have extraneous code? Leftover debug prints, private API keys etc...

- Does it have any obvious defects? Memory leaks, un-handled edge cases, security flaws, obsolete API calls, etc...

- Could it be more understandable? Add/remove abstractions, better variable/method names, more/less functional etc...

- Is the style consistent with the codebase and/or style guidelines?

- Are there obvious performance improvements? Hashset instead of list, lazy evaluations, etc...

- Is it sufficiently well tested?

I'm not even sure I agree that if I can't understand the code then it shouldn't go through. Some code is just really hard to understand. The aim is to make it as easy as possible to understand, while being functionally correct.