On Apr 10, 2004, at 10:17 AM, jpieper@tigris.org wrote:
> * subversion/libsvn_fs_fs/fs_fs.c
> (svn_fs__fs_open, svn_fs__fs_youngest_revision,
> svn_fs__fs_get_node_revision, svn_fs__fs_get_proplist,
> svn_fs__fs_rep_contents_dir, svn_fs__fs_rev_get_root,
> svn_fs__fs_revision_proplist): Impelemented at least partially
> and removed abort's on the control paths so implemented.
> (svn_fs__fs_file_length, svn_fs__fs_noderev_same_prop_key,
> svn_fs__fs_noderev_same_data_key): New
> (read_header_block, open_and_seek_revision, read_rep_line,
> get_fs_id_at_offset, get_root_offset): New helper routines that
> assist the above newly implemented functions.
I hate to be nitpicky, but the formatting in this file is pretty
random. There are a bunch of places with weird [*] if/else blocks,
some overly long lines, some sections that are indented too far, and
the 'space before the paren for function calls' seems to disappear and
reappear at random.
I've got a patch for all of this except the space before the paren
stuff, since I didn't know which direction this code was going to go on
that issue, but I'll hold off on committing it until after I hear from
you, since I'd hate to conflict with any more work you've been doing
here.
Also, how are you creating test cases for this? Manually? And from
what I can see it's just opening up the 'current' revision number. Is
that intended to exist forever, or is it temporary? I'm just curious
how we're going to atomically replace that file when commits occur...
Will it be a link of some sort?
Other than that, great stuff! It's nice to see some of the ideas for
libsvn_fs_fs solidifying into code.
-garrett
[*] something like:
if (foo)
{
blah ();
} else {
somethingelse ();
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Apr 10 17:56:16 2004