On Tue, Nov 13, 2018 at 04:25:49PM -0800, Elijah Newren wrote:
> This is a series of small fixes and features for fast-export and
> fast-import, mostly on the fast-export side.
I looked over this, and I think you've addressed all of my questions.
A few quick comments:
> Changes since v1 (full range-diff below):
> - used {tilde} in asciidoc documentation to avoid subscripting and
> escaping problems
I think just using backticks would make the source more readable, as
well as make the output prettier. But that's pretty minor.
> - renamed ABORT/ERROR enum values to help avoid further misusage
This is an improvement, I think. It's a little funny that we still have
bare names for the non-ABORT bits, though (there's less semantic
overlap, but if it's a good practice to use qualified enum names, we
should probably just do so consistently).
> - multiple small testcase cleanups (use $ZERO_OID, remove grep -A, etc.)
Looks good.
> - add FIXME comment to code about string_list usage
Makes sense.
> - record Peff's idea for a future optimization in patch 8 commit message
> (is there a better place to put that??)
Seems like a reasonable place (though you are welcome to restate it if
you like).
> - New patch (9/11): remove the unmaintained copy of fast-import stream
> format documentation at the beginning of fast-import.c
Looks good. I wondered if there might be bits that need migrated, but
given the length of time that comment has been there, it's unlikely. And
in the worst case, if somebody finds some information missing from
git-fast-import.txt, they can still consult the history.
> - Rewrite commit message for 10/11 to match the wording Peff liked
> better, s/originally/original-oid/, and add documentation to
> git-fast-import.txt
Looks good.
> - Rewrite commit message for 11/11; the last one didn't make sense to
> Peff. I hope this one does.
Thanks for your patience in getting me to understand what you're trying
to do. At this point I still think that using rev-list and diff-tree is
probably the right solution for your use case.
-Peff
