[noisy_boy]

I do opportunistic collection - when I am working on a feature, and spot opportunities for a refactoring/cleanup in code that is more or less directly related to the code I'm touching, I will keep making small incremental changes and keep testing them until my feature is implemented and the cleanup is done as well. I also ensure to not make a breaking change while doing this e.g. no change to the user facing api signature. If I see issues in unrelated code, that just gets a TODO and waits its turn when we have to do changes there for some feature request.

The reason is that I can atleast somewhat justify the changes under the umbrella of my feature while utilizing the allocated time budget. If I don't do this, we will never get a dedicated release to do tech cleanup - the backlog of feature requests is just too big and too little appetite on the decision makers' side for purely tech debt releases.

[sophiabits]

Big fan of this. Here in New Zealand we have a slogan “be a tidy kiwi” that encourages people to pick up their litter and be good stewards of for our natural environment

Imo the same mentality is good to have in software, and I’ve always appreciated being in a team that makes codebase improvements alongside feature additions. It makes things a lot more pleasant

[teeray]

I have done this in the past and it has worked well. On some teams though, I have been met with:

“Why did you do this refactor with the feature? Can you pull the feature into another PR, then we’ll leave the refactor in the original PR to be merged at another time? (read: never)”

[ralferoo]

I'd definitely agree with the putting the refactor into a separate PR and then the new functionality in another.

Aside from the obvious that people are more likely to be willing to review your change when it's small, it makes it much easier reason about both when they are separate.

There will be a lot of noise generated from the refactor that will drown out the new feature, but self-contained in its own PR it's a lot easier to understand the new feature and spot mistakes.

A simple refactor can likewise be likely skimmed over quickly, looking for the patterns of how it was done and assuming that most of the changes were similar and mostly just looking out for the differences.

As for accepting the feature PR but not the refactor PR, that suggests that the feature doesn't actually rely on the refactor. In that case, it's even clearer that they should be separated.

Personally, I'd always even create a new ticket for the refactor so that there's some justification for the work, and maybe you can say that this ticket blocks the one for the actual work. Maybe that's just me though, because I like to make sure that every non-trivial commit is tied to an issue in the bug/issue tracking software. This means that every bit of code is a simple "git blame" away from a justification of why it was changed.

[shepherdjerred]

A refactor should, at the very least, be in another commit if not another PR.

The refactor being separate in a separate commit/PR makes your code easier to review.

> Why did you do this refactor with the feature?

This indicates that the team doesn't think the refactor is needed or it wasn't a priority for them.

Teams that push back probably have a different mindset than you. Some teams don't understand the negative impact of tech debt, others don't know how to prioritize it.

[TeeMassive]

This is the only way I've found that we can do the needed refactor. As consultants, we never ask the permission of the client to refactor the code. Ever. That's not for them to decide. The reason they hired us in the first place is because we're the experts and they are not. We evaluate the needed refactor that puts the code in a decent state and put the time in the quote we provide the client. And even then we never create tasks or log time; when we find something that needs to be done, then our time is billed to the task relating to that issue.

[yarekt]

Consider a case where every non trivial refactor ends up in several rabbit holes, requiring lengthy meetings with lots of people to discuss, and many disagreements about direction. And in light of that the trivial refactors start to feel like you’re taking a bucketful out of a tsunami.

What ends up happening is it’s much more impactful for the team to just focus on shipping what they can, partly because they don’t have the tools/procedures/experience (anymore) for dealing with tech debt in this context

[noisy_boy]

And that is exactly how the cruft I see has built over many years (well before I joined the team). It is not that devs before me were not smart; they just focused on doing the bare minimum without any attention to keeping the code easier to manage and reason with. An append only garden.