3 ms·

“ Never fix a bug and refactor in the same pull request.” I’m sorry, but this is backwards. Bugs are many times caused by badly written code and you can tell w

“ Never fix a bug and refactor in the same pull request.” I’m sorry, but this is backwards. Bugs are many times caused by badly written code and you can tell when it’s the case. Refactoring the code many times fixes the issue without ever having to figure out where the needle was in the haystack. I guess in every field there are platitudes and prescriptions. At the end of the day, I try to follow first principles and ignore them and just focus on building a great product. What I see in this article is a disregard for the costs associated with context switching. My argument is, if you think you can handle the rabbit hole, and you think those related tasks will need to be done anyway at some point, head off to Wonderland. Because you have the context of the situation fresh in your temporary memory, so you’ll get it done faster than if you switch contexts and come back later.

mijoharas 4y ago
It also misses the point that sometimes refactoring makes it _easier_ to fix the bug, and that a large part of fixing a bug is understanding exactly what's happening with the code, which refactoring can also make easier. Overall I disagree with this article, both in it's definition of yak shaving (as being unrelated to what you're doing), and in it's assertion to never refactor and fix a bug in the same PR (now I'm not saying you should refactor things every time you fix a bug of course).
- mijoharas 4y ago
  I'm going to throw an addendum on to this one: if you fix a bug, and then do a refactor, I'll agree, it's probably best to split the PR's up. But that's just the common sense of keeping changes small and logically contained. I'll also agree that it's best not to interweave the two.
joshjje 4y ago
I think it should be more like, try to separate "formatting" and other changes into separate commits instead, you know like ones that change the whitespace all around, and other layout stuff, so diffs of the actual fix are easier later.
woojoo666 4y ago
Refactors usually take much longer than the bug fix, and while it acrues technical debt, there may be more urgent things to take care of. The article is about focusing on your initial goal, and then filing the refactor as a next step action item, instead of just growing scope endlessly.
- benstopics 4y ago
  I understand where you’re coming from. I would argue that if the amount of refactoring required to make the bug clear takes that much longer, then all the more reason it should be prioritized. This is really the purest definition of tech debt, because there may be other bugs present in the code you are unaware of. This is assuming no tests cover the bug, because if they did, it wouldn’t have made it into production. Honestly you should be doing it all, because you have to understand the full scope of the issue to properly fix the bug, test it to prevent regression, and in order to test it it must be testable. So I would say if there are no tests, and it is not testable code, the least amount of refactoring you should do is make it testable. You actually don’t even have to write the test if you really want to cut corners. Just through single responsibility principle, dependency injection, and writing code that could be tested is enough to bring it 90% the way there. You can even break the dependencies and theoretically as long as you don’t violate the interface the functions you refactored should hold up. The simpler and more broken down the code is, it gets to the point where you say, this function has one if statement and two return statements, writing a test is actually redundant compared to the code. If it’s not mission critical code, you can really cut corners, if you’re in a hurry…

mijoharas 4y ago

It also misses the point that sometimes refactoring makes it _easier_ to fix the bug, and that a large part of fixing a bug is understanding exactly what's happening with the code, which refactoring can also make easier. Overall I disagree with this article, both in it's definition of yak shaving (as being unrelated to what you're doing), and in it's assertion to never refactor and fix a bug in the same PR (now I'm not saying you should refactor things every time you fix a bug of course).

mijoharas 4y ago
I'm going to throw an addendum on to this one: if you fix a bug, and then do a refactor, I'll agree, it's probably best to split the PR's up. But that's just the common sense of keeping changes small and logically contained. I'll also agree that it's best not to interweave the two.

joshjje 4y ago

I think it should be more like, try to separate "formatting" and other changes into separate commits instead, you know like ones that change the whitespace all around, and other layout stuff, so diffs of the actual fix are easier later.

woojoo666 4y ago

Refactors usually take much longer than the bug fix, and while it acrues technical debt, there may be more urgent things to take care of. The article is about focusing on your initial goal, and then filing the refactor as a next step action item, instead of just growing scope endlessly.

benstopics 4y ago
I understand where you’re coming from. I would argue that if the amount of refactoring required to make the bug clear takes that much longer, then all the more reason it should be prioritized. This is really the purest definition of tech debt, because there may be other bugs present in the code you are unaware of. This is assuming no tests cover the bug, because if they did, it wouldn’t have made it into production. Honestly you should be doing it all, because you have to understand the full scope of the issue to properly fix the bug, test it to prevent regression, and in order to test it it must be testable. So I would say if there are no tests, and it is not testable code, the least amount of refactoring you should do is make it testable. You actually don’t even have to write the test if you really want to cut corners. Just through single responsibility principle, dependency injection, and writing code that could be tested is enough to bring it 90% the way there. You can even break the dependencies and theoretically as long as you don’t violate the interface the functions you refactored should hold up. The simpler and more broken down the code is, it gets to the point where you say, this function has one if statement and two return statements, writing a test is actually redundant compared to the code. If it’s not mission critical code, you can really cut corners, if you’re in a hurry…