{"thread":{"id":"64300","subject":"[PATCH v2] [Outreachy] commit.c: clarify comment describing commit re-parse behavior","startedAt":"2025-10-11T09:36:33Z","lastAt":"2025-10-14T00:41:28Z","messageCount":4,"participants":["Okhuomon Ajayi","Christian Couder","Jeff King"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"528566","messageId":"20251011093611.62937-1-okhuomonajayi54@gmail.com","threadId":"64300","inReplyTo":null,"subject":"[PATCH v2] [Outreachy] commit.c: clarify comment describing commit re-parse behavior","fromName":"Okhuomon Ajayi","fromEmail":"okhuomonajayi54@gmail.com","sentAt":"2025-10-11T09:36:11Z","receivedAt":"2025-10-11T09:36:33Z","isPatch":true,"sender":{"key":"okhuomonajayi54@gmail.com","avatar":null},"body":"The existing comment in `parse_commit_buffer()` mentioned a \"leftover\nfrom an earlier failed parse\", which could be confusing to new readers.\nIt implied an error state rather than the intended cleanup before\nre-parsing a commit.\n\nClarifying the comment makes it explicit that we reset the parent list\nto ensure a clean state before re-parsing, which improves readability\nand avoids misunderstanding during future maintenance.\n\nSigned-off-by: Okhuomon Ajayi \n---\n commit.c | 7 +++----\n 1 file changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex 16d91b2bfc..af20ca7c3d 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -475,10 +475,9 @@ int parse_commit_buffer(struct repository *r, struct commit *item, const void *b\n \tif (item->object.parsed)\n \t\treturn 0;\n \t/*\n-\t * Presumably this is leftover from an earlier failed parse;\n-\t * clear it out in preparation for us re-parsing (we'll hit the\n-\t * same error, but that's good, since it lets our caller know\n-\t * the result cannot be trusted.\n+\t * Reset the parent list before re-parsing to ensure a clear\n+\t * commit state. This avoids carrying over data from a previous\n+\t * incomplete or invalid parse.\n \t */\n \tfree_commit_list(item->parents);\n \titem->parents = NULL;\n-- \n2.43.0\n\n"},{"id":"528611","messageId":"CAP8UFD3nn=n3XLRKjrHpMOM4uf3wCFGMjdy13wOp=_vZHTeYWw@mail.gmail.com","threadId":"64300","inReplyTo":"20251011093611.62937-1-okhuomonajayi54@gmail.com","subject":"Re: [PATCH v2] [Outreachy] commit.c: clarify comment describing commit re-parse behavior","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2025-10-13T09:43:36Z","receivedAt":"2025-10-13T09:43:50Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Sat, Oct 11, 2025 at 11:36 AM Okhuomon Ajayi\n wrote:\n>\n> The existing comment in `parse_commit_buffer()` mentioned a \"leftover\n> from an earlier failed parse\", which could be confusing to new readers.\n> It implied an error state rather than the intended cleanup before\n> re-parsing a commit.\n>\n> Clarifying the comment makes it explicit that we reset the parent list\n\nWe use an imperative mood to describe what a patch does. (See the\n\"imperative-mood\" section of Documentation/SubmittingPatches.)\n\nSo maybe: \"Clarify the comment to make it explicit...\" or \"Let's\nclarify the comment to make it explicit...\"\n\n> to ensure a clean state before re-parsing, which improves readability\n> and avoids misunderstanding during future maintenance.\n>\n> Signed-off-by: Okhuomon Ajayi \n> ---\n\nWhen looking at your email on the mailing list archive:\n\nhttps://lore.kernel.org/git/20251011093611.62937-1-okhuomonajayi54@gmail.com/T/#u\n\nit looks like your message is the only one in its thread. So it's not\neasy to understand why it's a \"v2\", and what the corresponding v1 is.\n\nTo link a message to a previous one, there is the \"In-Reply-To:\" email\nheader. It can be added using the `--in-reply-to='...'` command line\noption if you use `git send-email` to send emails. (Not sure how to do\nit with GigGitGadget.)\n\nAt the very least, you could add a regular link to this part of your\nemail (after line containing only three dash characters above) maybe\nlike this:\n\nhttps://lore.kernel.org/git/20251010233303.783212-1-okhuomonajayi54@gmail.com/\n\nBy the way, in this part of your email patches, if there is no cover\nletter where you already do it, it's a good idea to explain the\ncontext of the patch, and when it's not the first version, to list the\nchanges compared to the previous version and often to provide a\nrange-diff. (See Documentation/SubmittingPatches about the cover\nletter.)\n\n> commit.c | 7 +++----\n> 1 file changed, 3 insertions(+), 4 deletions(-)\n\nThanks.\n"},{"id":"528678","messageId":"20251014003508.GD1507@coredump.intra.peff.net","threadId":"64300","inReplyTo":"20251011093611.62937-1-okhuomonajayi54@gmail.com","subject":"Re: [PATCH v2] [Outreachy] commit.c: clarify comment describing commit re-parse behavior","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-10-14T00:35:08Z","receivedAt":"2025-10-14T00:35:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Oct 11, 2025 at 10:36:11AM +0100, Okhuomon Ajayi wrote:\n\n> The existing comment in `parse_commit_buffer()` mentioned a \"leftover\n> from an earlier failed parse\", which could be confusing to new readers.\n> It implied an error state rather than the intended cleanup before\n> re-parsing a commit.\n> \n> Clarifying the comment makes it explicit that we reset the parent list\n> to ensure a clean state before re-parsing, which improves readability\n> and avoids misunderstanding during future maintenance.\n\nAs the original author of this comment, I think what you've written\nretains the intent but is easier to understand. So looks good to me.\n\n-Peff\n"},{"id":"528679","messageId":"CAFpMFfCKimM9zWODGgEnA962C+i4nCBL41JcRcX48yqwY+jtcQ@mail.gmail.com","threadId":"64300","inReplyTo":"20251014003508.GD1507@coredump.intra.peff.net","subject":"Re: [PATCH v2] [Outreachy] commit.c: clarify comment describing commit re-parse behavior","fromName":"Okhuomon Ajayi","fromEmail":"okhuomonajayi54@gmail.com","sentAt":"2025-10-14T00:41:16Z","receivedAt":"2025-10-14T00:41:28Z","isPatch":true,"sender":{"key":"okhuomonajayi54@gmail.com","avatar":null},"body":"Thanks a lot for the feedback, Christian and Jeff!\nI’ll reword the commit message to use the imperative mood and add a\nshort changelog with a link to the previous versions below the ‘---’\nline.\n\nOn Tue, Oct 14, 2025 at 1:35 AM Jeff King wrote:\n>\n> On Sat, Oct 11, 2025 at 10:36:11AM +0100, Okhuomon Ajayi wrote:\n>\n> > The existing comment in `parse_commit_buffer()` mentioned a \"leftover\n> > from an earlier failed parse\", which could be confusing to new readers.\n> > It implied an error state rather than the intended cleanup before\n> > re-parsing a commit.\n> >\n> > Clarifying the comment makes it explicit that we reset the parent list\n> > to ensure a clean state before re-parsing, which improves readability\n> > and avoids misunderstanding during future maintenance.\n>\n> As the original author of this comment, I think what you've written\n> retains the intent but is easier to understand. So looks good to me.\n>\n> -Peff\n"}]}