> > Clean up the SFP+ patch.
>
> Why send a patch and then immediately a cleanup? Why not
> just clean the
> original patch?
>
> > - if ((nesadapter->OneG_Mode) &&
> (nesadapter->phy_type[mac_index] != NES_PHY_TYPE_PUMA_1G)) {
> > + if ((nesadapter->OneG_Mode) && (nesadapter->phy_type[
> mac_index ] != NES_PHY_TYPE_PUMA_1G)) {
>
> This type of change isn't a cleanup... kernel style prefers
>
> array[index]
>
> to
>
> array[ index ]
>
> and it seems most of this patch is making the change to the
> less-good way?
>
> - R.
>
My bad, on the array index idiom. I can redo.
With regard to post patch clean-ups, I recall you telling me
that is was preferred to either front-load or back-load the
cleanups in a patch series.
I generally "cleaned-up" the entire functions rather than
just the patched portion. If I do both together, then you'll
get clean-up noise interspersed with functional deltas making
functional review somewhat annoying in my opinion.
Will be happy to redo as a single SFP patch
and drop 3rd patch if that works better for you. In fact that
is how I did it originally. :-)
Glenn
_______________________________________________
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
