Re: [net-next 11/13] igb: Update PTP function names/variables and locations.

[Joe Perches <joe@perches.com>]

On Thu, 2012-08-23 at 19:53 +0200, Richard Cochran wrote:
> On Thu, Aug 23, 2012 at 04:22:02PM +0000, Vick, Matthew wrote:
> 
> > > >  #ifdef CONFIG_IGB_PTP
> > > > -static int igb_ethtool_get_ts_info(struct net_device *dev,
> > > > +static int igb_get_ts_info(struct net_device *dev,
> > > 
> > > I like the old name better.
> > 
> > The old name is out of the coding style of igb. Every other
> > function is igb_get_* or igb_set_*, with the exception of
> > igb_ethtool_begin and igb_ethtool_complete.

> No, just leave the names alone, and keep the functions where they
> are. It is just churn.
> 
> One of the most useful ways to understand code (at least for me) is to
> use git blame. It tells you when code was added, what the reason was,
> and how the change looks in context. By moving and renaming willy
> nilly, you are obscuring this valuable information.

[]

> > I'm sorry you feel like this patch doesn't improve the driver.
> > The goal is code cleanup and consistency, both of which I
> > consider to be driver improvements and is why I made the patches. 
> 
> But the code wasn't dirty in the first place. It doesn't need this
> "cleaning." This series undoes the inline-able functions for no good
> reason. As far as ixgbe goes, this driver came first, so you might as
> well be making *that* driver consistent with this one.

I disagree with Richard.

Improving code clarity and consistency isn't churn.

Old code isn't necessarily the best code, nor should
necessarily old code be a required guide for new code.

The most valuable form of code is the current one,
not any antecedent version.

People that need to wade through old crud to blame
someone still can.