Hi Jonathan, Todd, Thank you for your replies. Yes, we are aware of the fact that submitting our patches as trunk patches first will be easier for the community. In fact, we have been trying to do so for a significant fraction of our recent patches, and it looks like that approach works relatively well for everyone.
However, submitting a patch on top of 89-fb first makes sense when the change requires live cluster testing on 89-fb clusters, e.g. pushing to dark launch, debugging GC issues, or doing A/B testing of regionserver code on a live cluster. In those cases, the workflow of "update 89-fb patch -> address comments -> build -> deploy to cluster -> fix bugs" is much more straightforward than "update 89-fb patch internally -> port changes to trunk -> update trunk patch -> address review comments -> port comments back to 89-fb -> build -> deploy to cluster -> fix bugs" which is approximately what we would have to go through if we were to conduct a code review for such a patch on top of the trunk. I agree that we could probably handle those reviews on a case-by-case basis and ask individual committers to review a particular 89-fb patch. Please let us know if you have any additional suggestions on how we could improve our development process to work better with the community. Thanks! --Mikhail On Mon, Feb 6, 2012 at 11:52 AM, Todd Lipcon <t...@cloudera.com> wrote: > Hi Mikhail, > > What you ask makes sense from your perspective but is difficult from > the community perspective. We're not familiar with your code base, so > it can be difficult to do a quality review on a non-trunk patch, > unless it's primarily new code. > > Perhaps when there is a large patch with mostly new code, you can ping > the dev list or a few individual committers to ask them to take a > look? > > -Todd > > On Mon, Feb 6, 2012 at 11:36 AM, Mikhail Bautin > <bautin.mailing.li...@gmail.com> wrote: > > Hello Everyone, > > > > Some of you have probably been wondering about what these "[89-fb]" > patches > > that our team submits for review are, so I would like to clarify that a > > little bit. We run a custom version of HBase based on 0.89 at Facebook, > > codenamed "0.89-fb", but we do our best effort to submit all of our > > improvements to the trunk as well. As a result, we frequently put an > 89-fb > > version of a patch for review first, go through a review loop, and only > > then put the trunk patch out for review. We have noticed that in such > > situations our trunk patches sometimes receive many more comments than > the > > earlier 89-fb versions of the same patches, which complicates our > > development workflow, because we have to go back and make these > additional > > changes as a follow-up patch to 89-fb. > > > > It would greatly simplify our workflow if people treated 89-fb patches > just > > like any other patches, and submitted most of their feedback on our code > > contributions (consisting of an 89-fb patch and a trunk patch) as part of > > whatever patch is published first. In other words, I would like to ask > you > > to treat 89-fb patches just the same as trunk patches, because a trunk > > patch is likely to follow. That was our hope when we open-sourced our > > internal version of HBase and moved our code review workflow to the > > externally-visible review system at http://reviews.facebook.net. The > only > > kind of 89-fb patches that we are not planning to port to trunk are > tagged > > [master], containing custom changes to the 89-fb master code. > > > > It would be great to hear what you think about the above and how we can > > make it easier for you to give us early feedback on our code > contributions. > > > > Thank you! > > --Mikhail > > > > -- > Todd Lipcon > Software Engineer, Cloudera >
