Hacker Newsnew | past | comments | ask | show | jobs | submitlogin
Ask HN: How to deal with low quality merge requests?
4 points by pestaa on April 11, 2016 | hide | past | favorite | 2 comments
We have a recurring problem with badly formatted code with stupid variable names, disgusting doc blocks, etc. One developer in particular tries to clean up his code line by line, wasting reviewers' time.

We're supposed to be in constant crunch mode, so coding guidelines are rarely written (they do exist however), and most office conventions are shared in verbal communication.

If the reviewers don't report problems on each and every line ("unneeded empty line", "clear up type hint", etc.), it remains inconsistent, ugly, or buggy.

I wonder how experienced engineers help their peers understand this problem better.



Whitespace and similar low hanging fruit should be flagged by scripts and fixed by submitter before reviews by humans take place.

As much as possible of other coding rules should be handled by scripts as well.

Talk to people and tell them to only submit merge request when they really believe they are done. If they need help before that it is better to ask coworkers for informal feedback on variable naming etc.

Added: If you are in 'constant crunch mode' then maybe you are wearing out your engineers and they just stop caring about quality?


And if the whole project isn't consistently formatted enough for your team to turn on a linter, then considering doing it only for incoming patches!




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

Search: