Re: [PATCH] commit: Add commit_list prefix to reduce_heads function.

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

 



Thiago Farina wrote:

> I think it's a good procedure for someone more familiar with this
> functions to do this. Perhaps, you or Junio?

If you are not familiar enough with the functions to document them
(perhaps with help from the list) then yes, renaming them is a bad
idea.  I am not inclined to do it because I like the current name.

The ideal patch is a great sort of present: first a bug report, then
the resolution to that bug.  When the patch proper goes awry, at least
there is the bug report.  I think you are trying to convey a bug but
you haven't explained it.  Maybe it is

 - "The reduce_heads function being used in various contexts, where it
   is not obvious what it means.  If you add commit_list to the name,
   then <such and such> becomes obvious.  So I suggest renaming."	or

 - "In my program, I have my _own_ reduce_heads function with
   different meaning so I cannot easily copy the commit_list functions
   to use them.  Please make it easier by putting commit_list functions
   in a well defined namespace."	or

 - "Some code is manipulating commit_lists directly and violating
   their invariants.  Please make it easier to build a cheat-sheet
   listing commit_list functions, to translate from
   bad-field-manipulation-ese to using-the-right-functions-ese."	or

 - "At my office there is a style guide indicating that each function
   should live in a module with some other functions and be named to
   indicate so (like perf, with its sched__* etc functions).  The idea
   is that code with a simple high-level structure tends to be easier
   to understand and we need to understand the code we use.  Can we
   start changing the code to fit this style guide, so there is less
   resistance to using it at my office?"

In a way, these are straw men; sorry about that.  The answer to each
would be different.  FWIW from my pov the answer to _none_ of these
would be "sure, let's rename the functions", for different reasons in
each case.

I do not think this is an atypical example at all.  I would have
prefered not to spend time on patches that require guessing what
problem is being solved.
--
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]