Roland Dreier wrote: > > Having waited for months for this patch to be merged in, it is very > disappointing > > to say the least. Wish it had been merged and if changes are needed they > can always be > > made subsequently. That has been my understanding of the development model. > > If you really want to get into it... > > I'll certainly accept some of the blame for taking too long to review > this patch. However, you didn't do yourself any favors by: a) making > one huge ugly patch and b) being rather disagreeable when someone > actually tried to review it. > > As far as the development model goes, it is certainly true that for > new things, we can merge first and fix later. But when we're touching > something like IPoIB, which is pretty critical to just about everyone > using the IB stack at all, the standard is a little different: we need > to be much more conservative. And even for new stuff, starting from a > good base is pretty important; it's easy to pick on coding style > problems, and indeed they do make review harder, but it's even more > important to have the underlying logic and structure be simple and > maintainable. > > Anyway, I'll post my current patch series shortly. I think I was able > to make the patch quite a bit neater and more reviewable: your patch > added > 400 lines, while the main part of my series adds < 200 lines. >
Roland, I realize a maintainers job is not easy with the incredible number of patches that are submitted as I have seen on this mailing list. And I am glad to see that this patch is starting to see some forward movement at long last :). I will review the patch and test it out and provide comments. Thanks for your efforts and help with this. There is some history behind why it became a huge ugly patch -which is not in the patch itself, and why I resisted making changes (before the merge). In the initial stages when I submitted the patch it got a very chilly reception and the message I got was "hands off my code". If you will recollect I did raise maintainability issues in the very beginning. But, from the communications I inferred that without incorporating those comments I could not get the patch in. There were a few genuine issues that the reviews pointed out. At the same time a lot of inconsequential comments were made too. Over time the patch morphed to incorporate several such comments. As you realize with a big patch, it is time consuming to test out every time the patch is changed and that too across multiple HCAs. Even though I was in agreement with Sean's comments (I had proposed several of them early on :) ) I deferred making those changes because they were undoing some of the changes suggested and I was not sure if there was agreement across the board. After all it had been reviewed by 3 people and continued to evolve. That is the reason I kept insisting that I would evaluate comments after the merge. Much easier to make small isolated changes after the big patch is in. Anyway, I would like to move on and close the chapter on for-2.6.24 tree since the window is now closed. Look forward to this patch being the first one to be merged into the for-2.6.25 tree. Pradeep Pradeep _______________________________________________ general mailing list [email protected] http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
