{"thread":{"id":"56256","subject":"[PATCH v1 0/1] traverse_trees_and_blobs: rename and tree-wide","startedAt":"2021-08-11T09:07:33Z","lastAt":"2021-08-12T19:24:20Z","messageCount":11,"participants":["Teng Long","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"432468","messageId":"cover.1628670288.git.dyroneteng@gmail.com","threadId":"56256","inReplyTo":null,"subject":"[PATCH v1 0/1] traverse_trees_and_blobs: rename and tree-wide","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2021-08-11T09:07:13Z","receivedAt":"2021-08-11T09:07:33Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"Teng Long (1):\n list-objects.c: traverse_trees_and_blobs: rename and tree-wide\n\n list-objects.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\n-- \n2.32.0.dirty\n\n"},{"id":"432469","messageId":"b6f3b15253d44c9250f59b53aafe471edda9f4b7.1628670288.git.dyroneteng@gmail.com","threadId":"56256","inReplyTo":"cover.1628670288.git.dyroneteng@gmail.com","subject":"[PATCH v1 1/1] list-objects.c: traverse_trees_and_blobs: rename and tree-wide","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2021-08-11T09:07:14Z","receivedAt":"2021-08-11T09:07:38Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"Function `traverse_trees_and_blobs` not only works on trees and blobs,\nbut also on tags, the function name is somewhat misleading. This commit\nrename it to `traverse_trees_and_blobs_and_tags`.\n\nSigned-off-by: Teng Long \n---\n list-objects.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/list-objects.c b/list-objects.c\nindex 7f404677d5..37a37fc502 100644\n--- a/list-objects.c\n+++ b/list-objects.c\n@@ -334,7 +334,7 @@ static void add_pending_tree(struct rev_info *revs, struct tree *tree)\n \tadd_pending_object(revs, &tree->object, \"\");\n }\n \n-static void traverse_trees_and_blobs(struct traversal_context *ctx,\n+static void traverse_trees_and_blobs_and_tags(struct traversal_context *ctx,\n \t\t\t\t struct strbuf *base)\n {\n \tint i;\n@@ -407,9 +407,9 @@ static void do_traverse(struct traversal_context *ctx)\n \t\t\t * needs a reallocation for each commit. Can we pass the\n \t\t\t * tree directory without allocation churn?\n \t\t\t */\n-\t\t\ttraverse_trees_and_blobs(ctx, &csp);\n+\t\t\ttraverse_trees_and_blobs_and_tags(ctx, &csp);\n \t}\n-\ttraverse_trees_and_blobs(ctx, &csp);\n+\ttraverse_trees_and_blobs_and_tags(ctx, &csp);\n \tstrbuf_release(&csp);\n }\n \n-- \n2.32.0.dirty\n\n"},{"id":"432501","messageId":"xmqqsfzf980v.fsf@gitster.g","threadId":"56256","inReplyTo":"b6f3b15253d44c9250f59b53aafe471edda9f4b7.1628670288.git.dyroneteng@gmail.com","subject":"Re: [PATCH v1 1/1] list-objects.c: traverse_trees_and_blobs: rename and tree-wide","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-08-11T18:00:00Z","receivedAt":"2021-08-11T18:00:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Teng Long writes:\n\n> Subject: Re: [PATCH v1 1/1] list-objects.c: traverse_trees_and_blobs: rename and tree-wide\n\nSorry but I do not quite get the \" and tree-wide\" part of the title.\n\n> Function `traverse_trees_and_blobs` not only works on trees and blobs,\n> but also on tags, the function name is somewhat misleading. This commit\n> rename it to `traverse_trees_and_blobs_and_tags`.\n\nThis looks pretty much \"Meh\" to me.\n\nThe current name might mislead people that we are limited to only\ntwo types, but I doubt the risk of misleading is worth reducing with\nsuch an overly long name. The original is long enough X-<.\n\nWhen we introduced it at 91904f56 (list-objects.c: factor out\ntraverse_trees_and_blobs, 2017-11-02), we may have been better to\ncall it traverse_non_commits(). The idea of traverse_commit_list(),\nwhich is its primary caller, is for its main loop to iterate over\ncommits and process them one by one in each iteration, and process\nobjects of other types discovered therein by calling this function\nonce per each commit [*1*].\n\ns/trees_and_blobs/non_commits/ will result in a name that is much\nshorter and to the point, I think.\n\n\n[Footnote]\n\n*1* The call to it at the end is meant to sweep anything\nleftover---we will not discover any tag while in the main loop of\nthe caller that iterates over commits (as they cannot contain any\ntag in there).\n\n\n\n> Signed-off-by: Teng Long \n> ---\n> list-objects.c | 6 +++---\n> 1 file changed, 3 insertions(+), 3 deletions(-)\n>\n> diff --git a/list-objects.c b/list-objects.c\n> index 7f404677d5..37a37fc502 100644\n> --- a/list-objects.c\n> +++ b/list-objects.c\n> @@ -334,7 +334,7 @@ static void add_pending_tree(struct rev_info *revs, struct tree *tree)\n> \tadd_pending_object(revs, &tree->object, \"\");\n> }\n> \n> -static void traverse_trees_and_blobs(struct traversal_context *ctx,\n> +static void traverse_trees_and_blobs_and_tags(struct traversal_context *ctx,\n> \t\t\t\t struct strbuf *base)\n> {\n> \tint i;\n> @@ -407,9 +407,9 @@ static void do_traverse(struct traversal_context *ctx)\n> \t\t\t * needs a reallocation for each commit. Can we pass the\n> \t\t\t * tree directory without allocation churn?\n> \t\t\t */\n> -\t\t\ttraverse_trees_and_blobs(ctx, &csp);\n> +\t\t\ttraverse_trees_and_blobs_and_tags(ctx, &csp);\n> \t}\n> -\ttraverse_trees_and_blobs(ctx, &csp);\n> +\ttraverse_trees_and_blobs_and_tags(ctx, &csp);\n> \tstrbuf_release(&csp);\n> }\n"},{"id":"432507","messageId":"YRQg+IJ3cxb3Vk+c@coredump.intra.peff.net","threadId":"56256","inReplyTo":"xmqqsfzf980v.fsf@gitster.g","subject":"Re: [PATCH v1 1/1] list-objects.c: traverse_trees_and_blobs: rename and tree-wide","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-08-11T19:11:52Z","receivedAt":"2021-08-11T19:11:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Aug 11, 2021 at 11:00:00AM -0700, Junio C Hamano wrote:\n\n> > Function `traverse_trees_and_blobs` not only works on trees and blobs,\n> > but also on tags, the function name is somewhat misleading. This commit\n> > rename it to `traverse_trees_and_blobs_and_tags`.\n> \n> This looks pretty much \"Meh\" to me.\n> \n> The current name might mislead people that we are limited to only\n> two types, but I doubt the risk of misleading is worth reducing with\n> such an overly long name. The original is long enough X-<.\n> \n> When we introduced it at 91904f56 (list-objects.c: factor out\n> traverse_trees_and_blobs, 2017-11-02), we may have been better to\n> call it traverse_non_commits(). The idea of traverse_commit_list(),\n> which is its primary caller, is for its main loop to iterate over\n> commits and process them one by one in each iteration, and process\n> objects of other types discovered therein by calling this function\n> once per each commit [*1*].\n> \n> s/trees_and_blobs/non_commits/ will result in a name that is much\n> shorter and to the point, I think.\n\nFWIW, I was about reply and suggest the exact same name. :)\n\nAs something internal to list-objects.c, I don't think it matters all\nthat much either way. The name \"traverse_commit_list()\" is IMHO more\nlikely to confuse. It is public within the project, and of course\ntraverses any type of object. So \"traverse_objects()\" or something may\nbe more accurate.\n\nOTOH I do not find it all that confusing, and it may not be worth the\ndisruption to the code base.\n\n-Peff\n"},{"id":"432565","messageId":"20210812075415.5114-1-dyroneteng@gmail.com","threadId":"56256","inReplyTo":"xmqqsfzf980v.fsf@gitster.g","subject":"Re: [PATCH v1 1/1] list-objects.c: traverse_trees_and_blobs: rename and tree-wide","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2021-08-12T07:54:15Z","receivedAt":"2021-08-12T07:54:34Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":">> Sorry but I do not quite get the \" and tree-wide\" part of the title.\n\nI may have some misunderstandings about \"tree-wide\".\nSorry, it will be resolved in the next patch.\n\n>>s/trees_and_blobs/non_commits/ will result in a name that is much\n>>shorter and to the point, I think.\n\nAgree and thanks for explanation.\n\nWill follow your suggestion in next patch.\n\nI don't know why I didn't think of this naming at the time (*headwalls*).\n\nIn fact, I discovered this problem while contributing packfile-uris\nrelated features (thinking about excluding tag objects), and finally\nsaid to myself \"So you are here\" (although it didn't take so much effort),\nSo I decide to make this patch.\n\n>>The call to it at the end is meant to sweep anything\n>>leftover---we will not discover any tag while in the main loop of\n>>the caller that iterates over commits (as they cannot contain any\n>>tag in there).\n\nSome doubts.\n\nI think at the object relationship level: tag> commit> tree> blob\n\nThe tag is on top because of the annotated tag. So, why the tag is\nin `revs->pending` to deal? I understand it should be the opposite?\nA commit should pending to a tag, at least equal.\n\nMaybe my understanding is wrong, if so, why this way?\n\nThank you.\n"},{"id":"432567","messageId":"20210812080542.5469-1-dyroneteng@gmail.com","threadId":"56256","inReplyTo":"YRQg+IJ3cxb3Vk+c@coredump.intra.peff.net","subject":"Re: [PATCH v1 1/1] list-objects.c: traverse_trees_and_blobs: rename and tree-wide","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2021-08-12T08:05:42Z","receivedAt":"2021-08-12T08:05:54Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":">>FWIW, I was about reply and suggest the exact same name. :)\n>>\n>>As something internal to list-objects.c, I don't think it matters all\n>>that much either way. The name \"traverse_commit_list()\" is IMHO more\n>>likely to confuse. It is public within the project, and of course\n>>traverses any type of object. So \"traverse_objects()\" or something may\n>>be more accurate.\n\nOn the basis of understanding the relationship between git objects,\n`traverse_commit_list` is understandable, for me.\n\n>>OTOH I do not find it all that confusing, and it may not be worth the\n>>disruption to the code base.\n\nAgree.\n\nIt’s interesting to read your reply, because of learning a lot of\nabbreviations.\n\nThank you.\n"},{"id":"432569","messageId":"cover.1628757762.git.dyroneteng@gmail.com","threadId":"56256","inReplyTo":"cover.1628670288.git.dyroneteng@gmail.com","subject":"[PATCH v2 0/1] list-objects.c: rename \"traverse_trees_and_blobs\" to \"traverse_non_commits\"","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2021-08-12T08:47:17Z","receivedAt":"2021-08-12T08:47:27Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"Teng Long (1):\n list-objects.c: rename \"traverse_trees_and_blobs\" to\n \"traverse_non_commits\"\n\n list-objects.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\nRange-diff against v1:\n1: b6f3b15253 ! 1: 2d19f0901c list-objects.c: traverse_trees_and_blobs: rename and tree-wide\n @@ Metadata\n Author: Teng Long \n \n ## Commit message ##\n - list-objects.c: traverse_trees_and_blobs: rename and tree-wide\n + list-objects.c: rename \"traverse_trees_and_blobs\" to \"traverse_non_commits\"\n \n Function `traverse_trees_and_blobs` not only works on trees and blobs,\n but also on tags, the function name is somewhat misleading. This commit\n - rename it to `traverse_trees_and_blobs_and_tags`.\n + rename it to `traverse_non_commits`.\n \n Signed-off-by: Teng Long \n \n @@ list-objects.c: static void add_pending_tree(struct rev_info *revs, struct tree\n }\n \n -static void traverse_trees_and_blobs(struct traversal_context *ctx,\n -+static void traverse_trees_and_blobs_and_tags(struct traversal_context *ctx,\n ++static void traverse_non_commits(struct traversal_context *ctx,\n \t\t\t\t struct strbuf *base)\n {\n \tint i;\n @@ list-objects.c: static void do_traverse(struct traversal_context *ctx)\n \t\t\t * tree directory without allocation churn?\n \t\t\t */\n -\t\t\ttraverse_trees_and_blobs(ctx, &csp);\n -+\t\t\ttraverse_trees_and_blobs_and_tags(ctx, &csp);\n ++\t\t\ttraverse_non_commits(ctx, &csp);\n \t}\n -\ttraverse_trees_and_blobs(ctx, &csp);\n -+\ttraverse_trees_and_blobs_and_tags(ctx, &csp);\n ++\ttraverse_non_commits(ctx, &csp);\n \tstrbuf_release(&csp);\n }\n \n-- \n2.32.0.1.g4c9ac18d93.dirty\n\n"},{"id":"432570","messageId":"2d19f0901ccb5ff27842b876cbe1085757a4aacc.1628757762.git.dyroneteng@gmail.com","threadId":"56256","inReplyTo":"cover.1628757762.git.dyroneteng@gmail.com","subject":"[PATCH v2 1/1] list-objects.c: rename \"traverse_trees_and_blobs\" to \"traverse_non_commits\"","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2021-08-12T08:47:18Z","receivedAt":"2021-08-12T08:47:37Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"Function `traverse_trees_and_blobs` not only works on trees and blobs,\nbut also on tags, the function name is somewhat misleading. This commit\nrename it to `traverse_non_commits`.\n\nSigned-off-by: Teng Long \n---\n list-objects.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/list-objects.c b/list-objects.c\nindex 7f404677d5..edaf1f5d64 100644\n--- a/list-objects.c\n+++ b/list-objects.c\n@@ -334,7 +334,7 @@ static void add_pending_tree(struct rev_info *revs, struct tree *tree)\n \tadd_pending_object(revs, &tree->object, \"\");\n }\n \n-static void traverse_trees_and_blobs(struct traversal_context *ctx,\n+static void traverse_non_commits(struct traversal_context *ctx,\n \t\t\t\t struct strbuf *base)\n {\n \tint i;\n@@ -407,9 +407,9 @@ static void do_traverse(struct traversal_context *ctx)\n \t\t\t * needs a reallocation for each commit. Can we pass the\n \t\t\t * tree directory without allocation churn?\n \t\t\t */\n-\t\t\ttraverse_trees_and_blobs(ctx, &csp);\n+\t\t\ttraverse_non_commits(ctx, &csp);\n \t}\n-\ttraverse_trees_and_blobs(ctx, &csp);\n+\ttraverse_non_commits(ctx, &csp);\n \tstrbuf_release(&csp);\n }\n \n-- \n2.32.0.1.g4c9ac18d93.dirty\n\n"},{"id":"432571","messageId":"cover.1628758416.git.dyroneteng@gmail.com","threadId":"56256","inReplyTo":"cover.1628757762.git.dyroneteng@gmail.com","subject":"[PATCH v3 0/1] list-objects.c: rename \"traverse_trees_and_blobs\" to \"traverse_non_commits\"","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2021-08-12T08:59:30Z","receivedAt":"2021-08-12T08:59:40Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"Patch-v3 fix the indent problem in v2.\n\n\nTeng Long (1):\n list-objects.c: rename \"traverse_trees_and_blobs\" to\n \"traverse_non_commits\"\n\n list-objects.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\nRange-diff against v2:\n1: 2d19f0901c ! 1: 8548aa8bb1 list-objects.c: rename \"traverse_trees_and_blobs\" to \"traverse_non_commits\"\n @@ list-objects.c: static void add_pending_tree(struct rev_info *revs, struct tree\n }\n \n -static void traverse_trees_and_blobs(struct traversal_context *ctx,\n +-\t\t\t\t struct strbuf *base)\n +static void traverse_non_commits(struct traversal_context *ctx,\n - \t\t\t\t struct strbuf *base)\n ++\t\t\t\t struct strbuf *base)\n {\n \tint i;\n + \n @@ list-objects.c: static void do_traverse(struct traversal_context *ctx)\n \t\t\t * needs a reallocation for each commit. Can we pass the\n \t\t\t * tree directory without allocation churn?\n-- \n2.32.0.1.g4c9ac18d93.dirty\n\n"},{"id":"432572","messageId":"8548aa8bb1ee89b9cce677c7fa35005fa15c5435.1628758416.git.dyroneteng@gmail.com","threadId":"56256","inReplyTo":"cover.1628758416.git.dyroneteng@gmail.com","subject":"[PATCH v3 1/1] list-objects.c: rename \"traverse_trees_and_blobs\" to \"traverse_non_commits\"","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2021-08-12T08:59:31Z","receivedAt":"2021-08-12T08:59:43Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"Function `traverse_trees_and_blobs` not only works on trees and blobs,\nbut also on tags, the function name is somewhat misleading. This commit\nrename it to `traverse_non_commits`.\n\nSigned-off-by: Teng Long \n---\n list-objects.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/list-objects.c b/list-objects.c\nindex 7f404677d5..11d97ac380 100644\n--- a/list-objects.c\n+++ b/list-objects.c\n@@ -334,8 +334,8 @@ static void add_pending_tree(struct rev_info *revs, struct tree *tree)\n \tadd_pending_object(revs, &tree->object, \"\");\n }\n \n-static void traverse_trees_and_blobs(struct traversal_context *ctx,\n-\t\t\t\t struct strbuf *base)\n+static void traverse_non_commits(struct traversal_context *ctx,\n+\t\t\t\t struct strbuf *base)\n {\n \tint i;\n \n@@ -407,9 +407,9 @@ static void do_traverse(struct traversal_context *ctx)\n \t\t\t * needs a reallocation for each commit. Can we pass the\n \t\t\t * tree directory without allocation churn?\n \t\t\t */\n-\t\t\ttraverse_trees_and_blobs(ctx, &csp);\n+\t\t\ttraverse_non_commits(ctx, &csp);\n \t}\n-\ttraverse_trees_and_blobs(ctx, &csp);\n+\ttraverse_non_commits(ctx, &csp);\n \tstrbuf_release(&csp);\n }\n \n-- \n2.32.0.1.g4c9ac18d93.dirty\n\n"},{"id":"432593","messageId":"YRV1YUEyaSn64VEV@coredump.intra.peff.net","threadId":"56256","inReplyTo":"8548aa8bb1ee89b9cce677c7fa35005fa15c5435.1628758416.git.dyroneteng@gmail.com","subject":"Re: [PATCH v3 1/1] list-objects.c: rename \"traverse_trees_and_blobs\" to \"traverse_non_commits\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-08-12T19:24:17Z","receivedAt":"2021-08-12T19:24:20Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Aug 12, 2021 at 04:59:31PM +0800, Teng Long wrote:\n\n> Function `traverse_trees_and_blobs` not only works on trees and blobs,\n> but also on tags, the function name is somewhat misleading. This commit\n> rename it to `traverse_non_commits`.\n\nThanks, this version looks good to me.\n\n-Peff\n"}]}