4

We are thinking of doing Continuous Integration with nightly builds here. Some of our sticking points is when to do code reviews. Is anyone out there doing Continuous Integration with nightly builds AND doing code reviews? If you can give a brief outline of your process that would be great. For example, here's what we are doing now:

  1. Developers attempt to check in code daily or as often as possible. Code does not have to be "done", it merely has to compile. They rebase their code with everyone else's changes at this time. Doing this often prevents us from merging nightmares.

  2. Builds come out nightly, but QA only looks at the builds but doesn't write up bugs yet, they merely inform us of what they are seeing.

  3. Every few days, we have an official release. Code is Reviewed. We write up a description of what is ready for testing, what isn't.

  4. QA writes up what they find in the stuff we say is ready for test. The process starts over.

So, what are we doing wrong? One thing I don't like, is that code is checked in that isn't reviewed. I heard before, that code should only be checked in that's been reviewed, however, I'm not sure how to do that and still have programmers check-in code daily. If you can give examples of what your process is, I'd really appreciate it?

Brian Tompsett - 汤莱恩
  • 357
  • 1
    The whole "Code is checked in that isn't reviewed" is a pretty dated statement at this point. I think you can make the argument that code that is deployed must be reviewed. But source control is too useful for works in progress to be blocked, its like saying you should not save a draft copy of a document. – Wyatt Barnett Jan 07 '12 at 23:13

1 Answers1

1

At work I've set up a process with Git and Gerrit and some custom CI scripts. Here's how it works:

  • All work is done on branches and merged into master later.
  • As developers work, they check in changes to Gerrit, where they are reviewed by others. Gerrit keeps the unreviewed changes in a separate holding area from the reviewed branch itself.
  • When a branch is complete, it's merged into master and then the merge commit is also submitted to Gerrit for approval.
  • The autobuild script builds the following: (a) master branch, (b) all reviewed work branches, and (c) any unreviewed code in Gerrit.

Gerrit manages the review process well and allows people to check in changes for review at any time, but they can't surreptitiously or accidentally change them after the review has passed.

Our environment is critical production software where all changes must be signed off before inclusion (that's why the merge commit goes through Gerrit as well). Separating the review of the changes from the merge commit review means that those two steps can be done at different times. Reviewing code doesn't necessarily mean it goes live.

Greg Hewgill
  • 10,231
  • Greg, in step 2 of your process, when the developer checks in the changes are those changes immediately available to other developers to merge into their code, or does it have to be approved first? –  Jan 07 '12 at 00:12
  • Other developers can get those changes if they want to; Gerrit makes them available through a special ref. Other developers working on the same branch might want to get them before review, and that's ok. Before they're reviewed, they aren't available through the "normal" branch unless you specifically go looking for them. – Greg Hewgill Jan 07 '12 at 00:29