6 ms·

How would this development style be compatible with code reviews?

jacques_chester 11y ago
It's not, in my view, unless you count pair programming as a sufficient substitute. I've seen branch-and-review used in anger. It can easily go bad: it drastically multiplies the inventory of work-in-progress. Everyone becomes blocked on someone else, moves on to another task, then loses context on the previous task or tasks. Stories with soft dependencies wind up sprinkled across several branches, making user testing a bit of a merge-and-pray event. It sounds great. Reviews! But efficient! And it turns out to be a tangle of interlocked gears. I imagine somebody has made it work. I have not seen such a case. My personal preference is a single development branch^, test-driven, pair programming, with rebase and integration test before pushing. [^] If it's a private project, this is master. On github, the master branch can't be used as a true master, because it is a de facto release branch.
- pvg 11y ago
  Isn't that a little at odds with the idea that the author is describing some variant of the OpenBSD development process (unless I misread the thing) and beside being a large and ongoing project, they've been reviewing code since before it became fashionable?
  - jacques_chester 11y ago
    The original literature on reviews that gets everyone hot and bothered describes something very different from what is done in practice. Formal meetings with scribes and reviewers and procedures and checklists are not the same as "here, check out this patch and merge it if you like it". I haven't kept up with research and I don't know if anyone has shown that TDD + pair programming is a reasonable substitute for full-blown code reviews in terms of bug yield. Now that I think of it, what I saw was an inversion of the classic CVS commit-bit model. CVS-based opensource projects typically have a praetorian guard who review everything before it gets committed. What I've generally seen is the reverse: soliciting opinions from other teams with little context or familiarity with the original codebase. So they take days or weeks to respond and when they do, it's quite cursory.
- 4lejandrito 11y ago
  Sounds to me that we work for the same company :) I bought into feature branches and all that stuff right after uni and tried to apply it at my first job without all the success I was expecting. Then when I moved to my current job in which we do TDD, Pair programming and single branch development I was a bit reluctant. After almost 2 years I haven't come up with a single scenario in which this approach has given us any troubles and in hindsight I think is what I should have proposed at my first job.
  - jacques_chester 11y ago
    Until I got to Pivotal Labs, developing on a single branch seemed impossible. Now it seems obvious. That said, I'm anchoring a project where my predecessors chose a feature-branch model. I have a motto that "sometimes it's better to be consistent than correct". Done properly, it's not too hard, but in practice that's because we don't have a large team and we still rebase on and merge back to master frequently.
    - ianamartin 11y ago
      That's a funny saying. I have a very different saying. It goes like this: "It's always better to be correct than consistent."
      - 4lejandrito 11y ago
        I will give you another one: When in Rome do as the Romans. I think consistency is a best practice with precedence over all the other ones. Having a team agreeing on something and being happy with it is sometimes far more important than that something being the best of the possible choices.
      - jacques_chester 11y ago
        Let me expand. The perfect is the enemy of the good. It's unethical for me to put my own standard of perfection ahead of delivering business value and soundly engineered software. My peers have opinions, formed validly, from their experiences. I have my own. Sometimes they differ. Sometimes I just need to suck that up and get on with the job.
ben_bai 11y ago
Big changes (mostly) need to be split into small and easy to review patches. There are almost no commits without at least one OK from another main developer.
glass- 11y ago
Developer changes code and create a diff. This diff is sent to the mailing list or other developers to test and review. The patch only gets committed when other developers have ok'd it.
yxhuvud 11y ago
Why would it be a problem? Actually it is favourable, since the reviewers always see the actual changes to the code base in each commit, without having to deal with merge commits.

It's not, in my view, unless you count pair programming as a sufficient substitute. I've seen branch-and-review used in anger. It can easily go bad: it drastically multiplies the inventory of work-in-progress. Everyone becomes blocked on someone else, moves on to another task, then loses context on the previous task or tasks. Stories with soft dependencies wind up sprinkled across several branches, making user testing a bit of a merge-and-pray event. It sounds great. Reviews! But efficient! And it turns out to be a tangle of interlocked gears. I imagine somebody has made it work. I have not seen such a case. My personal preference is a single development branch^, test-driven, pair programming, with rebase and integration test before pushing. [^] If it's a private project, this is master. On github, the master branch can't be used as a true master, because it is a de facto release branch.

pvg 11y ago
Isn't that a little at odds with the idea that the author is describing some variant of the OpenBSD development process (unless I misread the thing) and beside being a large and ongoing project, they've been reviewing code since before it became fashionable?
- jacques_chester 11y ago
  The original literature on reviews that gets everyone hot and bothered describes something very different from what is done in practice. Formal meetings with scribes and reviewers and procedures and checklists are not the same as "here, check out this patch and merge it if you like it". I haven't kept up with research and I don't know if anyone has shown that TDD + pair programming is a reasonable substitute for full-blown code reviews in terms of bug yield. Now that I think of it, what I saw was an inversion of the classic CVS commit-bit model. CVS-based opensource projects typically have a praetorian guard who review everything before it gets committed. What I've generally seen is the reverse: soliciting opinions from other teams with little context or familiarity with the original codebase. So they take days or weeks to respond and when they do, it's quite cursory.
4lejandrito 11y ago
Sounds to me that we work for the same company :) I bought into feature branches and all that stuff right after uni and tried to apply it at my first job without all the success I was expecting. Then when I moved to my current job in which we do TDD, Pair programming and single branch development I was a bit reluctant. After almost 2 years I haven't come up with a single scenario in which this approach has given us any troubles and in hindsight I think is what I should have proposed at my first job.
- jacques_chester 11y ago
  Until I got to Pivotal Labs, developing on a single branch seemed impossible. Now it seems obvious. That said, I'm anchoring a project where my predecessors chose a feature-branch model. I have a motto that "sometimes it's better to be consistent than correct". Done properly, it's not too hard, but in practice that's because we don't have a large team and we still rebase on and merge back to master frequently.
  - ianamartin 11y ago
    That's a funny saying. I have a very different saying. It goes like this: "It's always better to be correct than consistent."
    - 4lejandrito 11y ago
      I will give you another one: When in Rome do as the Romans. I think consistency is a best practice with precedence over all the other ones. Having a team agreeing on something and being happy with it is sometimes far more important than that something being the best of the possible choices.
    - jacques_chester 11y ago
      Let me expand. The perfect is the enemy of the good. It's unethical for me to put my own standard of perfection ahead of delivering business value and soundly engineered software. My peers have opinions, formed validly, from their experiences. I have my own. Sometimes they differ. Sometimes I just need to suck that up and get on with the job.

ben_bai 11y ago

Big changes (mostly) need to be split into small and easy to review patches. There are almost no commits without at least one OK from another main developer.

glass- 11y ago

Developer changes code and create a diff. This diff is sent to the mailing list or other developers to test and review. The patch only gets committed when other developers have ok'd it.

yxhuvud 11y ago

Why would it be a problem? Actually it is favourable, since the reviewers always see the actual changes to the code base in each commit, without having to deal with merge commits.