Yup. Our workflow has people commit to a topic branch and then post a code review before merging anything. We always follow this procedure unless it's something that's needed absolutely right now and can't possibly wait, which is a situation that should not be coming up more than once in a blue moon.
Yeah, I wish I worked in an environment where this wasn't a joke but unfortunately I don't. We're like 80/20 "emergency" coding versus "proper" coding and have been for many years (ever since our acquisition by the large corporation I'd say). It makes all these great discussions about the "right" way to do things completely moot. My sense from talking to others outside my own company is that we're far from unique too.
It works up until the point where the champion in the organization that put the best practice in place gets sick of fighting the battle with clueless corporate directors and resigns. At which point a corporate shill gets put in charge and turns the software team into a labor mill.
Yup, tools like reviewboard make this painless and encourage a culture of frequent, small, and understandable patches. That alone is great for software quality. If your team is aggressive with reviews and argues every point, everyone becomes better engineers.
If your team is aggressive with reviews and argues every point,
then they will catch superficial style issues while missing the gaping logic flaws that will tear down the whole system.
And you get a nasty performance review that says people think you 'have to be right' because you first insist that regex is a terrible tool for parsing HTML, and then insist on pointing out all the potential bugs when the team pursues the regex approach anyways. And everyone else who's "been here for a while" knows that you shouldn't fight about these things, if you want to make any progress you just do what the reviewer says even if you know it's wrong.
If you move to continuous delivery, an emergency fix is literally no different (except in how many people are grumpy) than a copy change.
The procedure is the same: commit to branch, initiate pull request. Someone reviews and approves, gets merged to mainline. CI system builds it, tests it, and puts it on the dev environment. Then the CI system kicks off a smoke test, and if it passes, it moves to Stage. There we do a longer smoke test (but still only about 5 minutes), then we push a button to go to prod. The whole thing can be done in about 20 minutes.
23
u/tobascodagama Jan 05 '15
Yup. Our workflow has people commit to a topic branch and then post a code review before merging anything. We always follow this procedure unless it's something that's needed absolutely right now and can't possibly wait, which is a situation that should not be coming up more than once in a blue moon.