Re: [PATCH 5/6] Add infrastructure to write revisions in fast-export format

Hi Jonathan,

Jonathan Nieder wrote:

The big downside is that as David mentioned it does not scale well with
the size of the directory and number of expansions.  But a patch is in
the pipeline to fix that.  I do not think it should hold the series up.

Right. The `dirents` branch is now merged.

Style: would probably be clearer to write:

       while (~(name = *path++)) {
               dirent = repo_dirent_by_name(dir, name);
               if (!dirent || !repo_dirent_is_dir(dirent))
                       break;
               dir = repo_dir_from_dirent(dirent);
       }

i.e., fewer unnecessary braces, and dealing with the exceptional cases
separately from the normal case.

This was (probably unintentionally) re-factored by David when merging
in his `dirents` branch.

As before, I wonder about the error cases.  Might it make sense to
report the error if someone tries to copy a nonexistent directory
from a previous revision?

Function is too long for my taste (I realize this is a matter of
taste).  The innermost blocks would make sense as functions in their
own right.

This came up in your previous review- I tried to split it up into more
functions, but due to the number of local variables, it turned out to
be a mess and I had to revert. Would you like to try re-factoring it?

My 80-column terminal is suffering.  Why not use the common
pattern?

Re-factored (again, probably unintentionally) by David during the merge.

These limits are not checked; is the caller supposed to check them
itself?  Does svn obey them?

I asked David too, and as far as we know, these limits are pretty
arbitrary. They're no enforced by SVN or any specific filesystem. We
can discuss about having less arbitrary limits and checking it in
svndump.c (while parsing the dump).

-- Ram