I’ve always been a big fan of improving code as I read it (though this opinion is controversial). This typically happens when I’m trying to understand some code I’ve never seen before (or code that I’ve written long enough ago to have forgotten it). If I see an opportunity to simplify or clarify the code, I want to do it right there on the spot. For one, it helps me to better understand the feature I’m working on. But also, if I don’t do it now, there’s a very small chance of it ever happening.
But there’s a downside.
When I commit the refactored code along with the new feature, it makes it hard to identify which parts of the check-in belong to the new feature and which are simply refactorings. If the commit causes a bug, then it becomes much more difficult to determine if it was the feature code that caused the issue or the refactoring. Additionally, it makes it harder for the project maintainer to review pull requests in a Fork & Pull environment, or any environment where commits must be reviewed before being accepted into the trunk.
A better solution (and the solution I currently use) is to create a separate branch for all refactoring related to a feature, and commit this separately from the feature branch. This has the advantage of keeping both types of work separate from the other. However, this also has a disadvantage: the feature branch cannot be committed without the refactoring branch if the feature branch is using the refactored code. In my case, when I’m refactoring to improve my feature, I want the refactoring in my feature branch. So, in this situation, the refactoring branch must be accepted first. If it is rejected, then the feature branch cannot be committed on its own.