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

[Glenn Streiff]

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

Hmm...what I probably should of done was given a clean sfp-patch
and then add peripheral cleanups to the functions as a subsequent
patch.  I'll go down that page.

Glenn