logoalt Hacker News

rstuart4133today at 3:22 AM1 replyview on HN

> I was mentored out of it, and while I still like my patches to be complete, I balance that with the available bandwidth of the team and what the team can reasonably actually process.

I struggle with the same issue. In my experience you can't reduce the total number of lines. If the feature took 10k or 15k loc to deliver it, you aren't going to be able to reduce that meaningfully.

You can usually break it into stacked series of commits. New code can be split up into stand alone modules, which all compile and pass their unit tests. They could even be shipped, although they wouldn't be used because the changes to the UI are always the last piece of the puzzle. If you are refactoring, you can usually find a way to split the refactor into smaller steps, each building on the previous one. That is almost certainly possible in this case.

The issue with both approaches is while you can review each step independently, what you miss by looking at just that commit is the motivation for doing it. You can only get that from the big picture, and to get the big picture you need the entire 10k or 15k loc available.

That means you have to push the entire series of commits. If you want to make it plain they are individually reviewable you push them as stacked PRs. Either way, it's a 15k loc push.

I don't see a way out of that for the same reason neither bottom up nor top down design works on their own. You have two edges - the upper (often the UI), and the lower (the OS, standard library - things you have to use to get anything done). You work from both edges simultaneously, each working towards the other, hopefully so that when they meet in the middle and the two fit together nicely. The point is, you have to review like that too. You can't just look at how neatly the blocks are stacked on the foundations, you have to evaluate if they are taking the best route to the destination. The review can fail in both ways - the UI can be beautiful but it stands on a mess, or the code could have built up a beautiful series of abstractions that bubble through to the top level and ultimately confuse the end user. So you have to review the code from both perspectives, and to do that you ultimately need to get your head around all 15k loc.

This means a reviewer demanding they be spoon fed a few thousand lines of code at a time is being as unreasonable as the person delivering 15k loc in one commit. They are demanding a simple solution to their problem, and it is wrong. They should be demanding all 15k loc be delivered in the form the author intends to ship, but split into digestible commits that clearly explain the path and reasoning taken between the top and bottom edges, so both the top and bottom level designs are plainly visible.

What happens when I do that is I get into fights over forced pushes. Everyone hates them, and for good reason. They asked for a simple change in their review, and what they want to see is a small commit reflecting their request. Hiding that by doing a rebase is met with howls of pain: "no forced pushes!". So you insert your commit reflecting just the change they asked for into the stack of commits the large feature necessitated. Doing that rebases every commit that depends on it, of course. You push the result and are treated with a chorus of "NO FORCED PUSHES".

Forbidding all forced pushes makes about as much sense as forbidding a 15k loc change, even through its well-structured into commits. It makes me wonder if unis bother to teach modern software engineering practices.


Replies

verandaguytoday at 8:53 PM

I will say that a good solution to this starts before a line of code is written, and does require a PM or scrum master with a deft touch (ideally one who's been involved with the engineering side of a project for a while).

Working with them, the scope can be brought down, sometimes, though this obviously depends a lot on externalities like who the customer is and how much flex they're comfortable adding to the timeline, since that often becomes a factor.

Part of becoming a more mature developer involves being able to navigate these situations with PMs better, and dealing with the frustrations that can often bring (in my experience). I'm still working my way up this side of the job. Historically this stuff has been managed up really thoroughly by my immediate manager (a factor of both the manager's working style, the work being done, and the broader company structure), but my current company structure means that I have to get better at this stuff a lot more actively.