[ofa-general] [GIT PULL] please pull infiniband.git
Pradeep Satyanarayana
pradeeps at linux.vnet.ibm.com
Mon Oct 29 11:03:45 PDT 2007
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
More information about the general
mailing list