Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Ship Small Diffs

blog.skyliner.io

Post reply on HN
Print view View original

41–50 of 76 posts

Re: Ship Small Diffs

#41

Post by nradov »

kornish wrote: post #14
davidkellis wrote: post #10

This strikes a chord with me, but I'm coming at it from a different angle. Does anyone else encounter very large diffs as standard operating procedure in golang codebases? Here I'm considering "very large" to mean 500+ lines changed - however github computes lines changed. I mean, I'm looking at this from the perspective of someone who contributes primarily to Ruby codebases. It's understandable that there would be a…

This definitely mirrors my experience on our Go codebase. Getting nontrivial features out the door generally requires hundreds of lines of code at a minimum, and sometimes thousands. We've taken to branching off of branches to keep code review moving quickly, but that creates new problems like cascading rebases all the way back down. If there's some secret to architecture or tooling (e.g. feature flags) that people u…

I work in an enterprise Java setting but I don't believe that this is a language specific issue. Our diff sizes vary by 3 orders of magnitude, so it would be meaningless to give an "average" line count. Each one is just large enough for a complete user story or defect fix, no more and no less. User stories have to be defined and broken down based on business value delivered to the customer regardless of how large or small the change is from an engineering standpoint.

Re: Ship Small Diffs

#42

Post by carterehsmith »

farnsworth wrote: post #37
carterehsmith wrote: post #24

>> Building a web application is a young and poorly-understood activity. Sorry, no. People have been shipping, and understanding how to ship web applications for... what, 10, 15, 20 years now?

And how many different tech stacks have we been through in that time? I build web apps using very different tools than I used 4 years ago. I read that sentence as comparing web dev on a timeframe to, say, civil engineering.

Yeah I can see how that is problematic.

What I do is -- look at what very big players are doing (Salesforce, Facebook, etc) and follow that.

Also, look at what all random minor players are doing, and absolutely ignore that.

Re: Ship Small Diffs

#43

Post by alexbecker »

jaibot wrote: post #29
grigjd3 wrote: post #25

Earlier quoted context omitted.

Compared to the history of agriculture, 20 years is nothing.

What about person-years?

I find it unimaginable that more people are coding than are farming full-time (which is at least hundreds of millions). Given that agriculture is also older by >2 orders of magnitude, I think it's pretty safe to say it has more person-years behind it.

Re: Ship Small Diffs

#45

Post by rtpg »

Groxx wrote: post #38

Ehh... nice in principle (and I do small deploys all the time for work), but too many artificially-small changes can easily cause you to "miss the forest for the trees". Each change is small and LGTM-able, but they can add up to a misbehaving system unless you have full context (which, because they're small, does not exist in the diffs). If it's conceptually a single unit, keep it a single unit. Pushing dead code in…

The problem is that review complexity is quadratic in size, and quadratic in number of interacting parts.

Each thing can interact with each other thing (in theory), so if you have double the diff, you might need 4 times the amount of work. And the increased workload increases the risk of a mistake.

But splitting up the diff in two doesn't change the result, right? The reality is that everything doesn't interact with everything else. But the reviewer doesn't know this, and needs to confirm non-interaction (hence a review).

If you, as an implementer, know the divisions, then shipping the pieces that are independent will do two things. First, it makes the review easier, letting it serve its main purpose: catching errors in implementation. Secondly, it will let _you_ confirm you know the divisions. It could be that your conception is wrong! Your small diff might not work because of an interaction you didn't realize.

Personally, I think that by the time you're sending diffs, the general architecture should have been decided. We tend to have at least one person (not the implementor) who knows how a feature is going to be implemented, so the architecture can be confirmed. You want someone to be looking at the forest! I think that doing this during code review is a bit harder though.

Taking the time to do it right is hard, and chopping things up makes it easier to do "it" (implementation review) right. Architecture review should probably be happening in a different space.

On a social level, you are asking for your code to be reviewed. Please have empathy for the reviewer.

Re: Ship Small Diffs

#46

Post by mcfunley »

carterehsmith wrote: post #40
mcfunley wrote: post #36

Earlier quoted context omitted.

Struggling with "how long have web applications existed" doesn't help your case that they are a solved problem.

Not sure what your point is. There is like 10 million people around that shipped a web app.

Ok, I'll give you a serious answer. For an individual shipping a website, anything at all works. I'd guess that half of all websites that have existed are just a single person editing files directly on a webserver without source control, and that's fine.

It's another matter altogether to ship code with tens of thousands of requests per second and a lot of teammates trying to change it simultaneously.

Re: Ship Small Diffs

#47

Post by [deleted] »

pavel_lishin wrote: post #34
scott_karana wrote: post #4

Earlier quoted context omitted.

`git revert` is perfectly suited for this, since it makes a new "inverse" commit of the changes. Your production deploys continue monotonically into the future as usual, and you now have version-controlled documentation of the rollback, instead of needing to maintain a separate mapping of production code state.

Doesn't this assume you only ever deploy one commit at a time?

[deleted]

Re: Ship Small Diffs

#48

Post by mcfunley »

rtpg wrote: post #45
Groxx wrote: post #38

Ehh... nice in principle (and I do small deploys all the time for work), but too many artificially-small changes can easily cause you to "miss the forest for the trees". Each change is small and LGTM-able, but they can add up to a misbehaving system unless you have full context (which, because they're small, does not exist in the diffs). If it's conceptually a single unit, keep it a single unit. Pushing dead code in…

The problem is that review complexity is quadratic in size, and quadratic in number of interacting parts. Each thing can interact with each other thing (in theory), so if you have double the diff, you might need 4 times the amount of work. And the increased workload increases the risk of a mistake. But splitting up the diff in two doesn't change the result, right? The reality is that everything doesn't interact with…

Great notes. Thanks for writing this!

Architecture review distinct from deploys is key, as is a separate operability review. And it's also important to mentally decouple the feature release to end users from the code releases.

Re: Ship Small Diffs

#50

Post by hyperpape »

nradov wrote: post #41
kornish wrote: post #14

Earlier quoted context omitted.

This definitely mirrors my experience on our Go codebase. Getting nontrivial features out the door generally requires hundreds of lines of code at a minimum, and sometimes thousands. We've taken to branching off of branches to keep code review moving quickly, but that creates new problems like cascading rebases all the way back down. If there's some secret to architecture or tooling (e.g. feature flags) that people u…

I work in an enterprise Java setting but I don't believe that this is a language specific issue. Our diff sizes vary by 3 orders of magnitude, so it would be meaningless to give an "average" line count. Each one is just large enough for a complete user story or defect fix, no more and no less. User stories have to be defined and broken down based on business value delivered to the customer regardless of how large or…

I'm in the same boat, but I feel like the problem is tying the commits to the user stories. Ideally you'd be able to make multiple commits to implement a single user story where it can't be done with a small amount of code.
Post reply on HN