refactoring is by definition changing the codebase without altering (logical) behavior, so you shouldn't need to differentiate between old and new
–
ratchet freakSep 5 '13 at 9:32

Assuming your CM Tool has update and merge functionality, I see no reason for any choice other than choice 1. Get the feature working on the feature branch. When it is time to add the feature to the main branch you simply do an update on the feature branch in order to pull in the updates to the main branch and then merge any conflicts. Retest the feature branch then merging it back into the main branch is trivial.
–
DunkSep 5 '13 at 15:00

1

@Dunk: That may make sense from a pure source-code perspective and indeed constitutes one of the major tenets of the GitHub model, but a successful merge/pull does not mean you haven't broken something important. That feature cannot be considered integrated until it's merged back into the mainline and passed all of the automated and manual tests. Most organizations that I've either worked with or been a part of don't actually have the cycles to support two active branches, although some try to downplay the integration/testing overhead and then spend 2 weeks on a bug bash.
–
AaronaughtSep 5 '13 at 15:15

@Aaron:Every change could potentially break something important. It doesn't matter if you are using no branches or 100 branches. The best one developer could do is merge the main branches into their branch and doing their own testing prior to their merging their changes into the main branch. On some projects (rare but has happened) we've had an integraton testing branch that developers merge into and another person verifies changes on this branch before promoting into the main branch. Still doesn't prevent missing something important.
–
DunkSep 9 '13 at 15:35

@Dunk: Nothing you can do will ever prevent defects, only reduce them. Several practices have been proven to reduce defects and save time over the long haul, including code review, unit testing, and continuous integration. Feature branches are not evil but they have to be handled carefully; long-running feature branches usually cause problems not because of merge conflicts but because they don't get sufficient review and testing before re-integration.
–
AaronaughtSep 9 '13 at 23:30

3 Answers
3

I'll try to post some helpful hints, but with all due respect, this is Release Management 101, and if you really have a huge code base and a need for parallel work streams in the organization, you would do well to either read a book on it or hire somebody with more experience in this area.

Assuming this situation:

The business needs new functionality that will take a long time to complete.

The business needs various bugs fixed now.

You have multiple developers who can work on different areas simultaneously.

You cannot defer the development of the feature until after the bugs are fixed.

Make absolutely sure that point #4 is true, because if you can invalidate that assumption, your job suddenly gets a whole lot easier. I can't tell you much time I've saved in the past by simply not doing bug-fixing and feature-development at the same time. That said, it is sometimes an unfortunate necessity.

Formally, your options are:

Feature Branches: You create a branch where all of the new feature development will take place. Not recommended because they inherently break Continuous Integration. You can of course clone your "CI build" for the feature branch, but that's not actually CI because the code hasn't actually been integrated, it's just being built and maybe tested in isolation.

Feature Toggles: This is typically not done with an ifdef but rather a configuration option. The ability to set this at runtime, not build time, is important because it makes testing and deployment a lot easier. Compile-time toggles require you to maintain two separate builds and two separate artifacts, but you can only promote one, a situation which tends to introduce ambiguity and process short-cutting.

Runtime feature toggles are partially recommended with some caveats:

It is important to minimize the surface area of the toggle. Ideally you would want the check to happen in only one place. Feature toggles that get too big for their britches are evil for the same reason that Singletons and other global state are. Try to check it just once, at application startup.

Too many feature toggles can become a spaghetti mess of their own over time, essentially creating a program-within-a-program. Feature Z depends on Feature Y which depends on Feature X but not Feature Q which is deprecated and breaks feature AB... etc. So in addition to minimizing the scope of individual toggles, you need to take steps to minimize the number of toggles and properly manage the dependencies between them. Even if there are no dependencies, you still need to run all end-to-end tests with each state that could plausibly be used in production, which quickly becomes a combinatorial explosion for the testers and release managers.

Parameterized Build or simply Multiple Targets: This is an incredibly useful tool/technique but I've never seen it used to build different feature sets and can't recommend that as an option here. You would normally use it to build for different platforms and/or perform certain one-off-tasks like running integration tests, generating documentation, etc. Using it as a poor-man's feature toggle has all of the potential drawbacks of a feature toggle and hides all of the complexity in your build scripts, which are generally the least flexible and most difficult to test of all your code.

Feature Abstraction, AKA Branch By Abstraction (a dangerous misnomer since it does not involve any actual branching): This means you encapsulate all of the interesting functionality of the new feature in a single class and use refactoring techniques (particularly Extract Interface) to encapsulate all of the existing functionality in the same interface. Then when you're ready to launch the feature, you just change the implementation.

This combines the advantages of (a) being in your trunk or mainline, (b) being fully covered by unit tests and possibly integration tests, and (c) not introducing any new global state to the application. This is much easier to do if your application uses dependency injection but not impossible if it doesn't. This is always the best choice if and when you are able to do it - it introduces the least overhead and the lowest risk, and can be combined very easily with options 2 or 3 above if you need to make the feature available on-demand to your testers.

Note that when using this option, once the feature has actually been launched, you should reevaluate whether the abstraction is still necessary and remove it if the overhead of maintaining it outweighs whatever benefits you're still getting.

You should choose options 4, 2, and 1 in descending order of preference. This is coming from someone with a whole lot of experience with all of the above options.

Final note: Fixing a bug or adding a new feature is not refactoring. Refactor does not mean "change", it refers to preserving the existing functionality exactly during some code change. So please don't use it to refer to your situation; what you're doing is the opposite of refactoring.

Off topic here I know.. but do you have a recommendation for said book relating to release management?
–
Simon WhiteheadSep 5 '13 at 13:59

+1, this is a great answer, much better than mine!
–
Doc BrownSep 5 '13 at 14:27

@SimonWhitehead: Probably the best high-level book I've read is Continuous Delivery by Humble and Farley. It can be a little on the dry and general side, but seeing all of the disparate concepts put together so neatly really turned my perception of release management on its head. Note, I'm working in a capital-A Agile organization and we do gated (QA-approved, sysadmin-deployed) releases of a highly-available, sorta-high-traffic website, so YMMV of course. Before that, I used to work on a 2-man team with no QA and developer-run deployments, and wouldn't have bothered with a lot of this.
–
AaronaughtSep 5 '13 at 15:10

@Aaronaught, thanks for the response. It is quite informative. I realized after some discussions that the number of files that will be changed during the feature development will be just a few. So, we see that 3 is most apt option as of now. And yes, I shouldn't have called it refactoring.
–
venkraoSep 5 '13 at 16:25

@venkrao: Up to you, but FWIW, note that #3 (build dependencies) is the only option I didn't recommend under any circumstances. It's never less effort than alternatives and almost always harder to maintain and track.
–
AaronaughtSep 5 '13 at 18:18

Option 2 is called "Feature Toggle". IMHO that is the option to prefer as long as the "feature entry point" is small, ideally just one #ifdef, not dozens. Even better is a feature toggle you can switch at run time, so you don't need a separate build run to test the current status of your new feature. This way, you can easier bring "intermediate releases" of your software into production, giving you earlier feedback if one of the changes accidentally broke an existing feature.

Option 1 is IMHO the second best option. It works better if you don't have a small entry point. It has the advantage that you have a lower risk of introducing any bugs into the current production version, but the drawback that typically the merging effort may be higher. Additionally, you will have to build and maintain two branches in parallel.

I don't know how well Option 3 is possible in your case, it is just a different form of Option 1, but it seems to me that this may cause some code duplication (within the same version of your software) - if that's the case, don't do it.

We feature toggle all the time for new features and even buggy features. A simple bit field in a database gets flicked on and suddenly an entirely new feature is available. Boss walks in and says "disable that button!" .. ~10 seconds to remove it.
–
Simon WhiteheadSep 5 '13 at 13:32

@SimonWhitehead: You're probably exaggerating, but if not, then I hope that this is internal business software you're talking about and that you aren't suggesting changing a production configuration for a public site in 10 seconds without following any kind of process...
–
AaronaughtSep 5 '13 at 13:44

@Aaronaught: all internal. We don't develop external software. Its not over-the-top.. it is just critical business processes / buggy processes. We are overhauling legacy code so feature toggle is a great way for us developers to introduce the updated business processes whilst continuing to maintain some of the legacy stuff. We release ~5 times a day.. so this really works well for us.
–
Simon WhiteheadSep 5 '13 at 13:53

if you have to modify the source that you have tagged, you will create a branch from that tag

About refactoring, the best way to be sure not to break existing functionalities is to write tets: unit tests, performance tests and integration tests. Run tests on a daily basis. Also continous integration is a good idea.

Is this functionally different from the OP's option 1? It looks exactly the same, except that you don't discuss any of the very important caveats (particularly the need to merge back into mainline and the perils of dangling release branches).
–
AaronaughtSep 5 '13 at 13:36

@Aaronaught it is the same, but described in more detail. About the caveats, I am not an expert of them and so I prefer not describing something that I could be wrong about.
–
Vitalij ZadneprovskijSep 5 '13 at 16:37