{"thread":{"id":"25968","subject":"[PATCH] commit: Add commit_list prefix to reduce_heads function.","startedAt":"2010-12-05T01:59:08Z","lastAt":"2010-12-06T04:01:30Z","messageCount":13,"participants":["Thiago Farina","Jonathan Nieder","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"157333","messageId":"a3f4bdc2d5f5d13c772a82de9afe2691b8a12863.1291514223.git.tfransosi@gmail.com","threadId":"25968","inReplyTo":null,"subject":"[PATCH] commit: Add commit_list prefix to reduce_heads function.","fromName":"Thiago Farina","fromEmail":"tfransosi@gmail.com","sentAt":"2010-12-05T01:59:08Z","receivedAt":"2010-12-05T01:59:08Z","isPatch":true,"sender":{"key":"tfransosi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/970071?v=4"},"body":"Signed-off-by: Thiago Farina \n---\n builtin/commit.c | 2 +-\n builtin/merge-base.c | 2 +-\n builtin/merge.c | 2 +-\n commit.c | 2 +-\n commit.h | 2 +-\n revision.c | 2 +-\n 6 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 4fd1a16..11a0412 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1321,7 +1321,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\t\t\tallow_fast_forward = 0;\n \t\t}\n \t\tif (allow_fast_forward)\n-\t\t\tparents = reduce_heads(parents);\n+\t\t\tparents = commit_list_reduce_reads(parents);\n \t} else {\n \t\tif (!reflog_msg)\n \t\t\treflog_msg = \"commit\";\ndiff --git a/builtin/merge-base.c b/builtin/merge-base.c\nindex 96dd160..9a8b445 100644\n--- a/builtin/merge-base.c\n+++ b/builtin/merge-base.c\n@@ -54,7 +54,7 @@ static int handle_octopus(int count, const char **args, int reduce, int show_all\n \tfor (i = count - 1; i >= 0; i--)\n \t\tcommit_list_insert(get_commit_reference(args[i]), &revs);\n \n-\tresult = reduce ? reduce_heads(revs) : get_octopus_merge_bases(revs);\n+\tresult = reduce ? commit_list_reduce_reads(revs) : get_octopus_merge_bases(revs);\n \n \tif (!result)\n \t\treturn 1;\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex c24a7be..de21499 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -821,7 +821,7 @@ static int finish_automerge(struct commit_list *common,\n \tif (allow_fast_forward) {\n \t\tparents = remoteheads;\n \t\tcommit_list_insert(lookup_commit(head), &parents);\n-\t\tparents = reduce_heads(parents);\n+\t\tparents = commit_list_reduce_reads(parents);\n \t} else {\n \t\tstruct commit_list **pptr = &parents;\n \ndiff --git a/commit.c b/commit.c\nindex 2d9265d..50bd85b 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -759,7 +759,7 @@ int in_merge_bases(struct commit *commit, struct commit **reference, int num)\n \treturn ret;\n }\n \n-struct commit_list *reduce_heads(struct commit_list *heads)\n+struct commit_list *commit_list_reduce_reads(struct commit_list *heads)\n {\n \tstruct commit_list *p;\n \tstruct commit_list *result = NULL, **tail = &result;\ndiff --git a/commit.h b/commit.h\nindex 9113bbe..648cadc 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -165,7 +165,7 @@ static inline int single_parent(struct commit *commit)\n \treturn commit->parents && !commit->parents->next;\n }\n \n-struct commit_list *reduce_heads(struct commit_list *heads);\n+struct commit_list *commit_list_reduce_reads(struct commit_list *heads);\n \n extern int commit_tree(const char *msg, unsigned char *tree,\n \t\tstruct commit_list *parents, unsigned char *ret,\ndiff --git a/revision.c b/revision.c\nindex b1c1890..a000d5d 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1795,7 +1795,7 @@ static struct commit_list **simplify_one(struct rev_info *revs, struct commit *c\n \t * Further reduce the parents by removing redundant parents.\n \t */\n \tif (1 < cnt) {\n-\t\tstruct commit_list *h = reduce_heads(commit->parents);\n+\t\tstruct commit_list *h = commit_list_reduce_reads(commit->parents);\n \t\tcnt = commit_list_count(h);\n \t\tfree_commit_list(commit->parents);\n \t\tcommit->parents = h;\n-- \n1.7.3.2.343.g7d43d\n"},{"id":"157334","messageId":"20101205021837.GA24614@burratino","threadId":"25968","inReplyTo":"a3f4bdc2d5f5d13c772a82de9afe2691b8a12863.1291514223.git.tfransosi@gmail.com","subject":"Re: [PATCH] commit: Add commit_list prefix to reduce_heads function.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-12-05T02:18:38Z","receivedAt":"2010-12-05T02:18:38Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Thiago Farina wrote:\n\n> Signed-off-by: Thiago Farina \n\nI know that the context is part of an effort to make the commit_list\nfunctions into something more of a self-contained API, but the\nreader does not know that. Perhaps you could say some words about\nthat in the change description: what's wrong with the current\nsituation, what context does this change come from, and what positive\neffect would it have?\n\nBeyond that, I must say I do not think this goes far enough to seem\nuseful. If I wondered what reduce_heads did, wouldn't\ncommit_list_reduce_heads be even more confusing? (ignoring the typo)\n\nPerhaps a more natural way to proceed would be as follows:\n\n . first, collect the functions to be treated as a module and\n list them in Documentation/technical (in this case, perhaps\n api-revision-walking or a new api-commit-list)\n\n . next, describe their current meaning. If this requires\n apologizing for the name, that's a good hint that a name\n change might be worthwhile\n\n . finally, tweak signatures (names and arguments) based on the\n results from step 2 and update the documentation at the same\n time.\n\nThat way, people used to the current functions would at least have\nsome documentation to help them adjust. What do you think?\n"},{"id":"157336","messageId":"7vsjycx05o.fsf@alter.siamese.dyndns.org","threadId":"25968","inReplyTo":"a3f4bdc2d5f5d13c772a82de9afe2691b8a12863.1291514223.git.tfransosi@gmail.com","subject":"Re: [PATCH] commit: Add commit_list prefix to reduce_heads function.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-12-05T05:24:03Z","receivedAt":"2010-12-05T05:24:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thiago Farina writes:\n\n> Signed-off-by: Thiago Farina \n\nI really do not like this.\n\nThe use of type \"struct commit_list\" to hold the set of parent commits is\nincidental; if we had \"struct commit_set\", we would have written a\nfunction with the same purpose, and named it the same \"reduce_HEADS\".\n\nAdding commit_list to the name makes the code harder to read (and type)\nwith little added benefit. \"LIST\"-ness is not the important part.\n\nIf a function takes a commit_list, named \"reduce_HEADS\", and returns a\ncommit_list, what it does should be obvious to you; otherwise you\nshouldn't be touching the internal of git.\n\nHaving said that, I do not claim \"reduce_heads\" is the world most\nwonderful short-sweet-descriptive name for what this function does and\nthere cannot be any better name. But commit_list_reduce_head is not it.\n\nIf the patch were to rename the function, especially the HEADs part, to\nclarify what it does, instead of how (iow, what type it uses to hold the\ndata) it does it, my reaction would have been very different.\n"},{"id":"157353","messageId":"AANLkTinAT3kotKQTS6eS1SLigNzSp6grAU7WNRbHf3N=@mail.gmail.com","threadId":"25968","inReplyTo":"20101205021837.GA24614@burratino","subject":"Re: [PATCH] commit: Add commit_list prefix to reduce_heads function.","fromName":"Thiago Farina","fromEmail":"tfransosi@gmail.com","sentAt":"2010-12-05T12:18:47Z","receivedAt":"2010-12-05T12:18:47Z","isPatch":true,"sender":{"key":"tfransosi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/970071?v=4"},"body":"On Sun, Dec 5, 2010 at 12:18 AM, Jonathan Nieder wrote:\n> Thiago Farina wrote:\n>\n>> Signed-off-by: Thiago Farina \n>\n> I know that the context is part of an effort to make the commit_list\n> functions into something more of a self-contained API, but the\n> reader does not know that.  Perhaps you could say some words about\n> that in the change description: what's wrong with the current\n> situation, what context does this change come from, and what positive\n> effect would it have?\n>\n> Beyond that, I must say I do not think this goes far enough to seem\n> useful.  If I wondered what reduce_heads did, wouldn't\n> commit_list_reduce_heads be even more confusing? (ignoring the typo)\n>\n> Perhaps a more natural way to proceed would be as follows:\n>\n>  . first, collect the functions to be treated as a module and\n>   list them in Documentation/technical (in this case, perhaps\n>   api-revision-walking or a new api-commit-list)\n>\nWhat you want here? That I describe the functions in these files? Why\nme? Why not the person who wrote them?\n\n>  . next, describe their current meaning.  If this requires\n>   apologizing for the name,\nApologize? For what? I don't understand what you mean here.\n\n> that's a good hint that a name  change might be worthwhile\n>\n>  . finally, tweak signatures (names and arguments) based on the\n>   results from step 2 and update the documentation at the same\n>   time.\n>\nI'd prefer to do just that step.\n\n> That way, people used to the current functions would at least have\n> some documentation to help them adjust.  What do you think?\n>\nI think it's a good procedure for someone more familiar with this\nfunctions to do this. Perhaps, you or Junio?\n"},{"id":"157354","messageId":"AANLkTikxibh4QkxzokhBYQ+dMS3W6PkDaDLuqm5qN+6v@mail.gmail.com","threadId":"25968","inReplyTo":"7vsjycx05o.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] commit: Add commit_list prefix to reduce_heads function.","fromName":"Thiago Farina","fromEmail":"tfransosi@gmail.com","sentAt":"2010-12-05T12:23:14Z","receivedAt":"2010-12-05T12:23:14Z","isPatch":true,"sender":{"key":"tfransosi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/970071?v=4"},"body":"On Sun, Dec 5, 2010 at 3:24 AM, Junio C Hamano wrote:\n> Thiago Farina writes:\n>\n>> Signed-off-by: Thiago Farina \n>\n> I really do not like this.\n>\nI don't feel very strong about it. And as I learned from Jonathan, I\ndon't care if you will take or not. I think my intention was good, but\nI can't please everybody\n\nI was just trying to put commit_list in a better shape and resemble it\nin a more explicit API.\n\n> The use of type \"struct commit_list\" to hold the set of parent commits is\n> incidental; if we had \"struct commit_set\", we would have written a\n> function with the same purpose, and named it the same \"reduce_HEADS\".\n>\n> Adding commit_list to the name makes the code harder to read (and type)\n> with little added benefit.  \"LIST\"-ness is not the important part.\n>\n> If a function takes a commit_list, named \"reduce_HEADS\",\n\nWhat? reduce_HEADS ? HEADS with CAPSLOCK?\n"},{"id":"157360","messageId":"20101205170919.GA7913@burratino","threadId":"25968","inReplyTo":"AANLkTinAT3kotKQTS6eS1SLigNzSp6grAU7WNRbHf3N=@mail.gmail.com","subject":"Re: [PATCH] commit: Add commit_list prefix to reduce_heads function.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-12-05T17:09:19Z","receivedAt":"2010-12-05T17:09:19Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Thiago Farina wrote:\n\n> I think it's a good procedure for someone more familiar with this\n> functions to do this. Perhaps, you or Junio?\n\nIf you are not familiar enough with the functions to document them\n(perhaps with help from the list) then yes, renaming them is a bad\nidea. I am not inclined to do it because I like the current name.\n\nThe ideal patch is a great sort of present: first a bug report, then\nthe resolution to that bug. When the patch proper goes awry, at least\nthere is the bug report. I think you are trying to convey a bug but\nyou haven't explained it. Maybe it is\n\n - \"The reduce_heads function being used in various contexts, where it\n is not obvious what it means. If you add commit_list to the name,\n then becomes obvious. So I suggest renaming.\"\tor\n\n - \"In my program, I have my _own_ reduce_heads function with\n different meaning so I cannot easily copy the commit_list functions\n to use them. Please make it easier by putting commit_list functions\n in a well defined namespace.\"\tor\n\n - \"Some code is manipulating commit_lists directly and violating\n their invariants. Please make it easier to build a cheat-sheet\n listing commit_list functions, to translate from\n bad-field-manipulation-ese to using-the-right-functions-ese.\"\tor\n\n - \"At my office there is a style guide indicating that each function\n should live in a module with some other functions and be named to\n indicate so (like perf, with its sched__* etc functions). The idea\n is that code with a simple high-level structure tends to be easier\n to understand and we need to understand the code we use. Can we\n start changing the code to fit this style guide, so there is less\n resistance to using it at my office?\"\n\nIn a way, these are straw men; sorry about that. The answer to each\nwould be different. FWIW from my pov the answer to _none_ of these\nwould be \"sure, let's rename the functions\", for different reasons in\neach case.\n\nI do not think this is an atypical example at all. I would have\nprefered not to spend time on patches that require guessing what\nproblem is being solved.\n"},{"id":"157361","messageId":"AANLkTikL4BWtzNgx1+MBYxRRdfL=Gu71KPjaiKXprvnb@mail.gmail.com","threadId":"25968","inReplyTo":"20101205170919.GA7913@burratino","subject":"Re: [PATCH] commit: Add commit_list prefix to reduce_heads function.","fromName":"Thiago Farina","fromEmail":"tfransosi@gmail.com","sentAt":"2010-12-05T17:29:51Z","receivedAt":"2010-12-05T17:29:51Z","isPatch":true,"sender":{"key":"tfransosi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/970071?v=4"},"body":"On Sun, Dec 5, 2010 at 3:09 PM, Jonathan Nieder wrote:\n>  - \"At my office there is a style guide indicating that each function\n>   should live in a module with some other functions and be named to\n>   indicate so (like perf, with its sched__* etc functions).  The idea\n>   is that code with a simple high-level structure tends to be easier\n>   to understand and we need to understand the code we use.  Can we\n>   start changing the code to fit this style guide, so there is less\n>   resistance to using it at my office?\"\n>\nFor me that is a good reason and I think it matches with what I had in\nmind but didn't write. Thanks for pointing it out.\n"},{"id":"157362","messageId":"AANLkTinjJpGW2OiXM3edWYaNhS+w4qNLrvg-0aBwsL=x@mail.gmail.com","threadId":"25968","inReplyTo":"AANLkTikL4BWtzNgx1+MBYxRRdfL=Gu71KPjaiKXprvnb@mail.gmail.com","subject":"Re: [PATCH] commit: Add commit_list prefix to reduce_heads function.","fromName":"Thiago Farina","fromEmail":"tfransosi@gmail.com","sentAt":"2010-12-05T17:32:18Z","receivedAt":"2010-12-05T17:32:18Z","isPatch":true,"sender":{"key":"tfransosi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/970071?v=4"},"body":"On Sun, Dec 5, 2010 at 3:29 PM, Thiago Farina wrote:\n> On Sun, Dec 5, 2010 at 3:09 PM, Jonathan Nieder wrote:\n>>  - \"At my office there is a style guide indicating that each function\n>>   should live in a module with some other functions and be named to\n>>   indicate so (like perf, with its sched__* etc functions).  The idea\n>>   is that code with a simple high-level structure tends to be easier\n>>   to understand and we need to understand the code we use.  Can we\n>>   start changing the code to fit this style guide, so there is less\n>>   resistance to using it at my office?\"\n>>\n> For me that is a good reason and I think it matches with what I had in\n> mind but didn't write. Thanks for pointing it out.\n>\n\nAlso I thought that as Junio already picked up the other patch. It's\nwas a hint that the other functions that has \"struct commit_list *l\"\nas its parameters could be renamed as well. But I was wrong it seems.\n"},{"id":"157370","messageId":"7vtyist0ko.fsf@alter.siamese.dyndns.org","threadId":"25968","inReplyTo":"AANLkTikxibh4QkxzokhBYQ+dMS3W6PkDaDLuqm5qN+6v@mail.gmail.com","subject":"Re: [PATCH] commit: Add commit_list prefix to reduce_heads function.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-12-05T20:40:55Z","receivedAt":"2010-12-05T20:40:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thiago Farina writes:\n\n>> If a function takes a commit_list, named \"reduce_HEADS\",\n>\n> What? reduce_HEADS ? HEADS with CAPSLOCK?\n\nI was just hiliting the relevant parts of the name for you. Read it as if\nit was painted in red or something.\n"},{"id":"157371","messageId":"7vk4joszcj.fsf@alter.siamese.dyndns.org","threadId":"25968","inReplyTo":"AANLkTinjJpGW2OiXM3edWYaNhS+w4qNLrvg-0aBwsL=x@mail.gmail.com","subject":"Re: [PATCH] commit: Add commit_list prefix to reduce_heads function.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-12-05T21:07:24Z","receivedAt":"2010-12-05T21:07:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thiago Farina writes:\n\n> Also I thought that as Junio already picked up the other patch. It's\n> was a hint that the other functions that has \"struct commit_list *l\"\n> as its parameters could be renamed as well.\n\nYou took a wrong hint and I think that is because you didn't think about\nwhat naming is for.\n\n\"insert-by-date\" does not say _why_ you want things to be inserted by date\n(neither \"sort-by-date\"). They are pretty generic looking names for any\nfunction that deal with a list of elements that record date. It makes\nsense to anticipate there will be many other such functions that deal with\ndifferent kinds of lists that hold date-recording things, and naming one\nof them \"this deals with list of COMMITS\" by saying \"commit_list_foo\"\nmakes quite a lot of sense, as \"insert-by-date\" does not give sufficient\ninformation to the reader.\n\nOn the other hand, \"reduce-heads\" is with quite a higher level of\nsemantics than \"insert-by-date\" and friends. The caller has a set of\ncommits and wants to remove the ones that can be reached by other commits\nin that set, typically because it wants to come up with a list of commits\nto be used as parents of a merge commit across them. It has much stronger\n\"why\" associated with it; unlike \"insert-by-date\" and friends, there can't\nbe many other such functions that deal with different datastructures that\nhold commits to reduce the heads the same way.\n\nA related tangent. There are two ways to name functions with richer \"why\"\ncomponent. Some people name them after what the caller expects them to do\n(e.g. they would name \"compute merge parents\" the function in question),\nand others names them after what they themselves do (e.g. it is about\nreducing the set of heads by removing redundant parents, and it does not\nquestion for what purpose the caller wants to do so). In general, it is\npreferrable to name them after what they do, not why the caller wants them\nto do so, especially when the semantics is clear. It will allow easier\nreuse of the function by new callers that do not create a new merge commit\nbut wants the same head reduction.\n"},{"id":"157372","messageId":"AANLkTi=QK=N+_iGR9-47JKFs_SDKujJ8c4mtnnM0yo94@mail.gmail.com","threadId":"25968","inReplyTo":"7vk4joszcj.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] commit: Add commit_list prefix to reduce_heads function.","fromName":"Thiago Farina","fromEmail":"tfransosi@gmail.com","sentAt":"2010-12-05T21:29:12Z","receivedAt":"2010-12-05T21:29:12Z","isPatch":true,"sender":{"key":"tfransosi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/970071?v=4"},"body":"On Sun, Dec 5, 2010 at 7:07 PM, Junio C Hamano wrote:\n> Thiago Farina writes:\n>\n>> Also I thought that as Junio already picked up the other patch. It's\n>> was a hint that the other functions that has \"struct commit_list *l\"\n>> as its parameters could be renamed as well.\n>\n> You took a wrong hint and I think that is because you didn't think about\n> what naming is for.\n>\n> \"insert-by-date\" does not say _why_ you want things to be inserted by date\n> (neither \"sort-by-date\").  They are pretty generic looking names for any\n> function that deal with a list of elements that record date.  It makes\n> sense to anticipate there will be many other such functions that deal with\n> different kinds of lists that hold date-recording things, and naming one\n> of them \"this deals with list of COMMITS\" by saying \"commit_list_foo\"\n> makes quite a lot of sense, as \"insert-by-date\" does not give sufficient\n> information to the reader.\n>\nThat makes sense to me. And clarified why the complain at all. And you\nare right.\n\nWould these be a candidates for adding commit_list_ prefix?\n\nfree_commit_list -> commit_list_free\ncontains\npop_most_recent_commit -> I'm not sure about this because of the\nlength of it, as Jonathan pointed in this thread.\npop_commit\n"},{"id":"157379","messageId":"7vbp4ztuwb.fsf@alter.siamese.dyndns.org","threadId":"25968","inReplyTo":"AANLkTi=QK=N+_iGR9-47JKFs_SDKujJ8c4mtnnM0yo94@mail.gmail.com","subject":"Re: [PATCH] commit: Add commit_list prefix to reduce_heads function.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-12-06T03:58:12Z","receivedAt":"2010-12-06T03:58:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thiago Farina writes:\n\n> Would these be a candidates for adding commit_list_ prefix?\n>\n> free_commit_list -> commit_list_free\n\nI think free_commit_list is a reasonable name for a function to free a\ncommit-list already.\n\n> contains\n\nHistorically I think we had two functions with this name, and both were\nnamed perfectly fine in the context they were introduced in.\n\nOne is \"does this haystack contain the needle we are looking for?\", which\nis a private helper in diffcore-pickaxe.c and considering what that module\ndoes, it is crystal clear that it is about \"needle in haystack\" without\nanything else tucked to its name.\n\nThe other is in 'pu' that came from Peff's \"How about this\" patch to\ncompute something similar to is-descendant-of more efficiently, while\nsacrificing the ability to be usable as a general helper function.\n\nAs I already said in the review of that stalled series, the particular\nimplementation of that function is good enough within the scope of the\ncommand it is used for (namely \"tag --contains\"); its implementation needs\nto be cleaned up, moved from commit.c to builtin/tag.c and made static to\nthe file, but as long as that happens, it is named appropriately.\n\n> pop_most_recent_commit -> I'm not sure about this because of the\n> length of it, as Jonathan pointed in this thread.\n> pop_commit\n\nThese two do not sound wrong---pop/push implies queue/list-ness and is\nquite clear that we are removing the topmost element from it.\n"},{"id":"157380","messageId":"7v7hfntuqt.fsf@alter.siamese.dyndns.org","threadId":"25968","inReplyTo":"AANLkTi=QK=N+_iGR9-47JKFs_SDKujJ8c4mtnnM0yo94@mail.gmail.com","subject":"Re: [PATCH] commit: Add commit_list prefix to reduce_heads function.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-12-06T04:01:30Z","receivedAt":"2010-12-06T04:01:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thiago Farina writes:\n\n>> \"insert-by-date\" does not say _why_ you want things to be inserted by date\n>> (neither \"sort-by-date\").  They are pretty generic looking names for any\n>> function that deal with a list of elements that record date.  It makes\n>> sense to anticipate there will be many other such functions that deal with\n>> different kinds of lists that hold date-recording things, and naming one\n>> of them \"this deals with list of COMMITS\" by saying \"commit_list_foo\"\n>> makes quite a lot of sense, as \"insert-by-date\" does not give sufficient\n>> information to the reader.\n>>\n> That makes sense to me. And clarified why the complain at all. And you\n> are right.\n\nActually I think s/insert_by_date/commit_list_insert_by_date/ is a\nmistake. Something like insert-commit-by-date would be more appropriate.\nSimilarly for s/sort_by_date/commit_list_sort_by_date/\n"}]}