git/list[1] front-page[2] threads[3] people[4] search[5] about
 

RE: [PATCH v3] fast-export: fix regression skipping some merge-commits

From
ICIsaac Chou <isaac.chou@microfocus.com>
Date
Jun 1, 2018, 19:41 UTC
Message-ID
<MW2PR18MB22845EE9631BB6586BA4E8E8E5620@MW2PR18MB2284.namprd18.prod.outlook.com>
In-Reply-To
<CAN0heSqQckJT-4mHbwoP_XjbE2-UH8+k2nG6mnN76_t3nzO_xw@mail.gmail.com>
Hello, I need help on this topic again.  I need to inform our customers what release this issue will be addressed in.  I checked the 2.17.1 binary release recently and found that the fix is not included.  Can someone help me with that information or point me to a document that I can use to determine it myself?
Thanks,
Isaac
-----Original Message-----
From: Martin Ågren [mailto:martin.agren@gmail.com] 
Sent: Saturday, April 21, 2018 3:00 AM
To: Junio C Hamano <gitster@pobox.com>
Cc: Git Mailing List <git@vger.kernel.org>; Johannes Schindelin <johannes.schindelin@gmx.de>; Isaac Chou <Isaac.Chou@microfocus.com>; Jonathan Tan <jonathantanmy@google.com>
Subject: Re: [PATCH v3] fast-export: fix regression skipping some merge-commits
On 21 April 2018 at 05:43, Junio C Hamano <gitster@pobox.com> wrote:
Show 28 quoted lines
> but I do not think the updated "fix" below is better.  It might be 
> just aesthetics and I suspect I won't find it as disturbing if we 
> could push with
>
>         object_array_push(commits, (struct object *)commit);
>
> or something that is more clearly symmetric to object_array_pop().
> The "Queue again" comment is needed only because use of "add"
> highlights the lack of symmetry.
>
> With add_object_array(), it looks somewhat more odd than your previous
>
>         peek it to check;
>         if (it should not be molested)
>                 return;
>         pop to mark it consumed;
>         consume it;
>
> sequence, in which peek() and pop() were more obviously related 
> operations on the same "array" object.
>
> And I do not think it is a good idea to introduce _push() only for 
> symmetry (it would merely be a less capable version of add whose name 
> is spelled differently).  Hence my preference for peek-check-pop over 
> pop-oops-push-again-but-push-spelled-as-add.
>
> Not worth a reroll, though.  I just wanted to spread better design 
> sense to contributors ;-)
Thanks for your wise words. :-) One thing that just occurred to me is that if the original site where we `add_object_array()` all objects starts adding a non-NULL `name` for some reason, then we need to remember to do the same with this new caller. I suspect that at that time, at the latest, we will be switching to peek-check-pop.
Thanks for sharing your thoughts.
Martin
Previous: Martin ÅgrenNext: Duy Nguyen
Message 17 of 18 in “RE: [BUG] Git fast-export with import marks file omits merge commits”
  1. Isaac ChouApr 19, 2018
  2. Elijah NewrenApr 19, 2018
  3. Junio C HamanoApr 19, 2018
  4. Martin ÅgrenApr 20, 2018
  5. Isaac ChouApr 20, 2018
  6. fast-export: fix regression skipping some merge-commitsMartin Ågren, Apr 20, 2018
  7. Isaac ChouApr 20, 2018
  8. fast-export: fix regression skipping some merge-commitsMartin Ågren, Apr 20, 2018
  9. Johannes SchindelinApr 20, 2018
  10. Martin ÅgrenApr 20, 2018
  11. Johannes SchindelinApr 20, 2018
  12. fast-export: fix regression skipping some merge-commitsMartin Ågren, Apr 20, 2018
  13. Eric SunshineApr 20, 2018
  14. Martin ÅgrenApr 21, 2018
  15. Junio C HamanoApr 21, 2018
  16. Martin ÅgrenApr 21, 2018
  17. Isaac ChouJun 1, 2018
  18. Duy NguyenJun 2, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.