Hacker News Reader

Comment by chunkyks | Hacker News Reader

Fond memories of the time I used "git bisect" to find a bug, and the commit that introduced it was some breathtakingly-large codebase-reorganisation-but-shouldn't-have-changed-anything-functionally.

Thankfully the commit didn't have my name at the top. I made him fix it.

View on HN

> Thankfully the commit didn't have my name at the top. I made him fix it.

EDIT: To make it more explicit than the original passive-aggressive comment I initially wrote (below): I think we need to stop worrying about writing perfect, big-less code, but instead acknowledge that anyone can make mistakes during refactoring and we should never leave that responsibility alone.

What happened to collaboration, support, and blaming the lack of automated tests instead of the person doing the hard work of a zero-velocity (but highly valued) refactoring?

The person who knows the change and thinks that it shouldn't have changed anything is probably in the best position to figure out why it broke something after all.

Probably refactoring according to a pattern, and missed something. Still, the person with context gets the bug.

Eh, depends on what you’re optimizing for. For fixing the bug as quickly as possible, yes. For spreading that context throughout the team or getting other features out that the person with context is best equipped to ship, no.

If it is just reorganisation of project files then the fastest way to fix the bug would likely be to just revert the commits.

Consequently, this highlights why the first person may have an incentive to fix it: if their code gets reverted to fix the issue, they will definitionally be forced to rework it before they’re able to merge again.

This can very much depend on your company 'culture' (for lack of a better word) if you ask me.

Depending on the company you might be in one of a few situations and not in all of them should you blame something else.

For example, this person might have actually put this PR up with a description akin to "I finally did the big xyz refactoring we've all been wanting to do for a long time but never dared. I checked a, b and c and d as well, had Peter from QA do some exploratory tests around the areas we were most afraid of and I _think_ we should be OK. I know it's a huge change set but please take and extra careful look. It _should_ be OK but ya know now I've alerted Murphy". Senior people actually gave it a good look in code review and it was finally merged. Shit happens. This guy totally deserved collaboration, help and not blame. Agreed.

Now second scenario: the guy that's known as 'the cowboy' around the company merged yet another ultimately objectively useless and simply opinionated refactoring that 'should change nothing' and got his buddies to OK the PR without even looking. Lo and behold this refactoring also went up in flames causing a dumpster fire yet again. This guy deserves nothing but to be made to fix this up all by himself. Maybe he will finally learn. If he doesn't it might be a good idea to part ways with him. If the company can't do that and protects him maybe it's time for the good guys to go to a company that deserves them instead.

this isn't blame. you broke it, you fix it. this is how people learn from mistakes. do you ridicule the person? absolutely not. does it belong to the team? absolutely. But, you broke it, you fix it. See also, "Pottery Barn Rule"

On large codebases that does not hold because you can't expect everybody to know all the code and how it should behave. If you broke it but there was no test, it's not your fault.

"if you liked it, then you should have put a test on it".

I was vaguely flippant in my post and it lacked nuance. Actually this project [and my workplace in general] is very friendly and collegial, he was happy to do it, he even already knew where to look since I had done the bisect and otherwise triaged the rest of the issue. We collaborated on an even larger-scale refactoring earlier this year.

Mainly my point was the first paragraph; that bisect is awesome right up until the issue is in a ten thousand line commit.

Reply on news.ycombinator.com