[ofa-general] RE: [ PATCH 3/3 ] RDMA/nes SFP+ cleanup

Glenn Streiff gstreiff at NetEffect.com
Tue Apr 29 10:16:11 PDT 2008


 
>  > 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



More information about the general mailing list