On Mon, Nov 12, 2012 at 4:45 PM, Jonathan Nieder <jrnieder@xxxxxxxxx> wrote: > Max Horn wrote: > >> Aha, now I understand what this patch is about. So I would suggest >> this alternate commit message: >> >> remote-testgit: make it explicit clear that we use the 'done' feature >> >> Previously we relied on passing '--use-done-feature ' to git >> fast-export, which is easy to miss when looking at this script. > > I'm not immediately sure I agree this is even a problem. Is the point > that other fast-import frontends do not have a --use-done-feature > switch, You mean other fast-exports. And what other fast-exports? Most remote helpers don't use an external fast-export tool, and the only I know that used one is the one I wrote (and is now deprecated) that used bzr fast-export, and no, that one didn't support the done feature. Most remote helpers would probably be doing the equivalent of fast-export themselves. > so a typical remote helper has to do that work itself, and the > sample "testgit" remote helper would be a more helpful example by > doing that work itself? Yes. > The idea behind --use-done-feature is that if fast-export exits early > for some reason and its output is going to a pipe then at least the > stream will be malformed, making it easier to catch errors. So there > is something to be weighed here: is it more important to illustrate > how to make your fast-export tool's output prefix-free, What fast-export tool? This is a remote helper. > or is it more > important to illustrate how to work around a fast-export tool that > doesn't support that feature? Ditto. If you want to launch a campaign of adding the 'done' feature to whatever fast-export tools are out there (that I'm not aware of), go ahead, but this is about remote helpers, most (all?) of which would not use a fast-export tool to achieve the export, but do it themselves. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html