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

Re: [PATCH v2 3/4] fast-export: don't handle uninteresting refs

From
Felipe Contreras <felipe.contreras@gmail.com>
Date
Oct 31, 2012, 01:39 UTC
Message-ID
<CAMP44s0RcbAiUmvGACxO+H-b-anQSPXxUqUuZwYRKWfrpXYeew@mail.gmail.com>
In-Reply-To
<20121031010823.GX15167@elie.Belkin>
On Wed, Oct 31, 2012 at 2:08 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
Show 7 quoted lines
> Felipe Contreras wrote:
>
>> I don't think it's my job to explain to you how 'git fast-export'
>> works.
>
> Actually, if you are submitting a patch for inclusion, it is your job
> to explain to future readers what the patch does.
That's already explained.
> Yes, the reader
> might not be deeply familiar with the part of fast-export you are
> modifying.

This has nothing to do with what you said. I'm literally explaining to you how 'git fast-export' works in situations that are completely orthogonal to this patch, because you are using wrong examples as grounds to prevent this patch from being accepted. It's not my job to explain to you that 'git fast-export' doesn't work this way, you have a command line to type those commands and see for yourself if they do what you think they do with a vanilla version of git. That's exactly what I did, to make sure I'm not using assumptions as basis for arguing, it took me a few minutes.

That being said, if your problem is that it's not clear to people not deeply familiar with that part of fast-export, this extra paragraph in addition to the current commit message should do the trick:

--- The reason this happens is that before traversing the commits, fast-export checks if any of the refs point to the same object, and any duplicated ref gets added to a list in order to issue 'reset' commands after the traversing. Unfortunately, it's not even checking if the commit is flagged as UNINTERESTING. The fix of course, is to do precisely that. ---

And to get that all had to do is ask: "Can you please add an explanation of what this part of the code does? For the ones of us not familiar with it".

Not; "This patch looks unsafe", "This patch makes Sally mad", "This patch causes regressions", and so on.

But hey, at least we are not arguing about what is wrong with this patch (or so I hope).

Cheers.
-- 
Felipe Contreras
Previous: Jonathan NiederNext: Jonathan Nieder
Message 23 of 29 in “Re: [PATCH v2 4/4] fast-export: make sure refs are updated properly”
  1. Sverre RabbelierOct 30, 2012
  2. Felipe ContrerasOct 30, 2012
  3. Sverre RabbelierOct 30, 2012
  4. Felipe ContrerasOct 30, 2012
  5. Jonathan NiederOct 30, 2012
  6. Felipe ContrerasOct 30, 2012
  7. Sverre RabbelierOct 30, 2012
  8. Felipe ContrerasOct 30, 2012
  9. Sverre RabbelierOct 30, 2012
  10. Felipe ContrerasOct 30, 2012
  11. Jonathan NiederOct 30, 2012
  12. Jonathan NiederOct 30, 2012
  13. Jonathan NiederOct 30, 2012
  14. Felipe ContrerasOct 30, 2012
  15. Felipe ContrerasOct 30, 2012
  16. Jonathan NiederOct 30, 2012
  17. Felipe ContrerasOct 30, 2012
  18. Jonathan NiederOct 30, 2012
  19. Felipe ContrerasOct 30, 2012
  20. Jonathan NiederOct 30, 2012
  21. Felipe ContrerasOct 31, 2012
  22. Jonathan NiederOct 31, 2012
  23. Felipe ContrerasOct 31, 2012
  24. Jonathan NiederOct 31, 2012
  25. Felipe ContrerasOct 31, 2012
  26. Jonathan NiederOct 31, 2012
  27. Felipe ContrerasOct 31, 2012
  28. Jonathan NiederOct 31, 2012
  29. Johannes SchindelinOct 30, 2012

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.