Hacker News new | ask | show | jobs
by avmich 1283 days ago
Pull requests which have artificial requirements. Which, these days, may mean many things.

It's illusion that PR can usually be compact in terms of lines and files. That not only requires a good quality of code, which isn't ubiquitous, but also good understanding of code, which changes all the time with different opinions resulting in that code being modified. So PRs become rather large - and sometimes surprisingly missing useful things, because exact change isn't easy.

The PR which is hard to review may mean that the code isn't understood the same way by different people, and this is a signal that some discussion is needed. Unfortunately often that's not done, and after some efforts PR is pushed through, while misunderstandings grow.

1 comments

"Reduce the scope of your PR" a reviewer comments on a PR for a project that hasnt meaningfully addressed its technical debt in years.
I think this may be a valid point, and cleaning up could be seen as a separate concern.

The frustrating part is ALWAYS needing to clean up before making any contribution.

So following the advice, you always need two PRs.

Depends on the cleanup no? I mean, if it's just obselete version of one library, and the ensuing lockfile change, it doesn't really matter. If iys upgrading to a new language/framework however, yes, please do so.
I'm not saying cleaning up separately isn't warranted.

But it isn't always legacy that needs cleaning up.

It can also be mess created very recently by active team members.

So on the one hand, a separate PR for the cleaning up is warranted, but since it isn't being matched with a separate PR for introducing the mess, the double standard is magnified by introducing more book-keeping for those who clean up after others.

For that reason, when working with messy people who are not held accountable, I usually clean up in separate commits in a PR. This way, when looking at the blame for the logical changes that I introduced, and when reviewing, this is separate from the cleanup.

My last team was like this. So painful