Re: [PATCH 2/3] Name make_*_path functions more accurately

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On miÃ, 2011-03-16 at 12:29 -0400, Brian Gernhardt wrote:
> On Mar 16, 2011, at 12:06 PM, Carlos MartÃn Nieto wrote:
> 
> > Rename the make_*_path functions so it's clearer what they do, in
> > particlar make clear what the differnce between make_absolute_path and
> > make_nonrelative_path is by renaming them real_path and absolute_path
> > respectively. make_relative_path has an understandable name and is
> > renamed to relative_path to maintain the name convention.
> > 
> > Signed-off-by: Carlos MartÃn Nieto <cmn@xxxxxxxx>
> 
> I didn't try it, but it looks like 2/3 horribly breaks the code and
> 3/3 fixes it.  I personally (and I think others) prefer patches that
> are each useful on their own.  Especially since a code-breaking patch
> like this makes bisecting harder.

 True enough.

> 
> I would suggest doing one of the following:
> 
> 1) Squashing 2/3 and 3/3 so all the renaming occurs at once.
> 2) Adding wrappers from the old name to the new in 2/3 and removing
> them in 3/3.

 I'll squash.

> 
> That said, I'm not sure the renaming is useful although the
> documentation comments definitely are.

 Do you think the difference between make_nonrelative_path and
make_absolute_path is clear without looking at the code? For me at
least, a relative path is the opposite of an absolute one, and a
non-relative path is the opposite of a relative one. To make a
difference between absolute and non-relative is then bound to lead to
errors.

   cmn

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]