25

At my company, all of our development (bug fixes and new features) is done on separate branches. When it's complete, we send it off to QA who tests it on that branch, and when they give us the green light, we merge it into our main branch. This could take anywhere between a day and a year.

If we try to squeeze any refactoring in on a branch, we don't know how long it will be "out" for, so it can cause many conflicts when it's merged back in.

For example, let's say I want to rename a function because the feature I'm working on is making heavy use of this function, and I found that its name doesn't really fit its purpose (again, this is just an example). So I go around and find every usage of this function, and rename them all to its new name, and everything works perfectly, so I send it off to QA.

Meanwhile, new development is happening, and my renamed function doesn't exist on any of the branches that are being forked off main. When my issue gets merged back in, they're all going to break.

Is there any way of dealing with this?

It's not like management will ever approve a refactor-only issue so it has to be squeezed in with other work. It can't be developed directly on main because all changes have to go through QA and no one wants to be the jerk that broke main so that he could do a little bit of non-essential refactoring.

10
  • What version control are you using? There are different approaches for DVCS and a centralized server model. Furthermore, what are the development branches being taken off of? If a feature branch is accepted, how do other dev branches pick up the changes?
    – user40980
    Oct 16, 2013 at 20:59
  • 2
    As an aside, a diagram of the current branching structure could be really helpful. It is quite possible that the root of the problem with the difficulty with the refactoring is in part caused by some... unconventional branching policies (see programmers.stackexchange.com/questions/210360 for one such example). I would also suggest reading vance.com/steve/perforce/Branching_Strategies.html to get some ideas and background (if I am able to answer this question that will be a major reference point).
    – user40980
    Oct 16, 2013 at 21:42
  • 1
    Last paragraph sums it up - if the Business does not perceive the value there is not way a major refactor can go ahead. You need to work with your test team to resolve their timelines. (I suspect your QA is really Test in drag (They put a wig and lipstick on and pretend to be something they are not). A real Q.A. team would be telling you what to refactor, not getting in your way. )
    – mattnz
    Oct 16, 2013 at 22:30
  • 1
    @mattnz: You're quite right. They're not a real QA team. They're mostly customer support. I think a lot of their responsibilities should be shifted back over to the Dev team because they simply can't handle everything we dump on them, but that's a management issue and a battle I've yet to win.
    – mpen
    Oct 16, 2013 at 22:47
  • 3
    You missed my dig. Test != QA. QA oversees quality, and aims to improve business outcomes. Test attempts to prove the absence of defects by finding them.
    – mattnz
    Oct 17, 2013 at 2:00

7 Answers 7

12

There are several problems that are mixing together to make refactoring challenging in this environment. Mixed into this are some non-technical problems ("but that's a management issue and a battle I've yet to win").

The first problem to look at is the long running branch. These branches have difficulty with tracking changes outside of the view of the developer. To address this:

  • When the code is complete - give it a once over (let customer support look at it if they want), but merge it quickly into develop so that other changes that depend it will be able to be picked up and changes that conflict are identified early in the process.
  • If, for some reason a brach does become long running while refactoring is going on, it tends to be good practice to merge from stable into the branch to pick up changes and refactoring. Often this minimizes conflicts and surprises on merge from the feature branch into the stable branch.
  • All out integration testing needs to be done on releases - not features. In this environment features may or may not be fully integrated with the system. While it is possible to do a sanity check on the feature in isolation, it doesn't identify issues upon the release.
  • From time of code completion to merge to (lets call it develop - branching from master / stable / release has its own issues of not picking up the latest development changes) shouldn't be too long. The longer you wait, the more knowledge that is lost and the harder it is for the code to become integrated with other code lines.

Another issue that is mixing into this is that I alluded to with the above points is the changing role of the branch over time. It starts out as a development branch where developers commit, and then becomes a testing area (what testing is being done here that can be meaningful in the whole of the application?), which is then merged into stable (and presumably released - is it tested again?).

With a shorter feature start to end time it is easier for the refactoring to be able to be picked up by other branches.

Encourage developers to get the entire environment. Just cherry-picking changes can lead to... lets say interesting developer environments. While cherry-picking has its uses, for that to be the default mode of pulling changes into a branch can be worrisome.

Refactoring is something that ideally is done constantly, or if not constantly whenever there is a modicum of downtime. Branch, do a simple refactoring, run the unit tests to verify everything is still working (its unit tested, right? right?) and then merge back into stable. Pass around the information for other developers to pull those changes that you refactored into their own branches.

It is important for developers to own the quality of the code. While the direction of the features comes from outside, and the time allocations are often not our own, code quality is something that it is necessary to take pride in and make time for.

You may find the following questions useful in the quest for allocating time for dealing with technical debt:

You may also wish to look at tools such as sonar which can help identify the areas of the code that need the most work for refactoring. The technical debt plugin is something that can be used to help point out the accumulation of debt over time in the code base.

Often it is necessary to point out that the ROI for dealing with technical debt is a faster turnaround time for features and bug fixes from the development team.

1
  • Tests are essentially performed at three points in time. Once when the issue is claimed fixed (to make sure it meets all the requirements and there are no major problems), again when it is merged back into default (integration testing), and again when we do a build (integration with all cherry picked issues/final look over). I think cherry-picking is necessary in our environment as we operate a SaaS with very particular clients. I'll take a look at these links, thanks for the pointers! Edit: There's actually one more look-over on production to make sure it went up OK.
    – mpen
    Oct 17, 2013 at 0:42
3

Usually I am developing refactored version in "parallel" with current, i.e. in same codebase, but not referencing it from core application. And when new solution is done and tested, I am starting actual refactoring.

Example 1. Assume I have Thing, let it be either function, interface, module, or whatever. And I want to refactor it. I am creating Thing2 in same codebase, it is refactored version of Thing. When it is done and tested, I am refactoring everything that references Thing, to replace it with Thing2. Usually this step takes relatively small amount of time.

If actual refactoring takes too much time to keep in sync without screwing team, I am taking all relevant features, and refactoring them in parallel, too.

Example 2. I have new rendering backend, that is refactored version of old one. But it is not compatible with old rendering frontend. Thus, I need to refactor frontend. And again: in same codebase. When everything is done, I am just changing class of frontend instance, ideally it will take one short commit.

Yes, recursively one may come to conclusion that everything must be done in parallel. But this usually happens when there is too much coupling in codebase, or it is changing too fast.

Finally, when new code integrated and works well, old features may be removed from codebase, and new features may be renamed to get old names.

Generally, idea is to prepare new features in parallel and switch to using them by one small step.

John Carmack uses this (or at least similiar) approach, perhaps his blog post explains it better: (link)

1
  • This is a good approach. I'm trying to remember what really prompted this question now... I don't think it was something very amenable to parallelization. Or if it was, I think my concern is that this approach causes a lot of fragmentation in the codebase. We have "old ways" of doing things and "new ways" of doing things, and the old stuff is being replaced at a glacial pace but it's causing headache for developers because now they essentially have to know two (or more) systems.
    – mpen
    Oct 17, 2013 at 21:10
1

It may look like a difficulty in the technical side when actually it's in the requirements side.

Where the development is oriented towards different requirements in different branches is the real difficulty. The managers and architects of the team should make decisions that may enable the different Business needs to co-exists.

ZBB process and Co-Dev "compromises" when done after making right decisions with relevant inputs from all the developers should afterwords enable you the implement what you need without having to think - How will I merge my code.

ZBB stands for Zero-based budgeting. By saying Co-Dev I meant few people that are working in parallel programming.

2
1

Your problem is the branch model you're using. You could develop on a branch, and when complete and ready for QA, the branch gets merged to an 'intermediate trunk', sometimes called Integration or Test. When you develop the next feature, you can branch from this intermediate trunk instead.

This model allows you to develop multiple features in parallel on different branches, merging them all together onto the Integration branch to send to QA, and also maintain a single trunk of releases (you merge the codebase QA received to the main trunk when they certify it)

You are making an assumption that your changes delivered to QA will be passed without major modification though - if the QA code comes back with instructions to remove half the changes, you'll have to revert but if that doesn't happen it'll make your development much smoother. So you're basically taking branches for new features off what your mainline code will be (ie trunk after merging in code passed to QA), rather than what it is today (ie current trunk) and so no longer developing against the previous release's codebase.

1

The problem seems to me that you are working excessively long on branches. Cost of conflicts grows exponentially with the length everyone stays on a branch, so with very long conflicts you have little chance of doing any refactoring.

0

Having since joined a different company, I can confirm this is a process issue, not a tooling issue.

The proper way to approach a refactor is:

  1. Have lots of automated tests[1]
  2. Make your refactor as small as possible, modifying as few files as you can while still making progress towards your goal[2]
  3. Run the tests to ensure you didn't break anything. If this is a pure refactor, you shouldn't need to update any tests.
  4. Submit/merge into mainline.

None of this should go through QA/manual testing, but it is a good idea to get your code reviewed by another software developer.

By making your changes small, you reduce the chance of merge conflicts. It also allows you to submit quicker. By making your refactors pure, you're not changing anything user-visible, so it shouldn't need to go through QA.

If you work for a company that doesn't allow refactors, sit down with them and explain the importance of code health.


[1]: If you don't have them, write them. They're important.

[2]: For example, you can alias your class or function to desired name and deprecate the old version without removing it yet. Then in follow up "branches" you can update a few of the callers to the new version. Submit/merge and repeat.

0

My rule: You never merge into a release branch. Instead, you merge the release branch into your branch, solve all the conflicts, do everything needed to make your branch acceptable (pull requests wit review, QA, automated tested and do on) and at that point you push to your release branch, which will NoT do any merging - your branch will completely replace what’s in the master branch.

The next person wanting to move their branch to the release branch will do exactly the same. Merge release into their own branch (including all your changes), solving conflicts etc.

2
  • You're using "merge" imprecisely here. I'm pretty sure the spirit of your answer is correct but "merging" is not the issue here. The opposite of "merge" is not "push". The distinction is between pushing or pulling; and merging or rebasing. If anything, pushing onto a main branch is the thing you should avoid, instead favoring pull requests. During the completion of said pull request, you either merge or rebase. From experience, this is a case where merging is actually preferable, as it both avoids a higher sensitivity to conflicts and it leaves a readable history of the merge chronology.
    – Flater
    Mar 21, 2022 at 10:01
  • To finish my thought, the reason a merge is preferable here is because a rebase would intermingle your feature branch commits based on when each individual commit happened. But this can lead to a lot of conflicts if you e.g. complete the PR of a branch with older commits after you complete the PR of a branch with more recent commits. It's actually better here to merge, because then you change the content of the main branch in the same chronology that you approved PRs; which tends to be the same chronology in which you resolved merge conflicts as well. That's why it makes the most sense.
    – Flater
    Mar 21, 2022 at 10:05

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.