On Apr 10, 2004, at 2:22 PM, Josh Pieper 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.
>
> Sorry about that. Subversion's coding style is radically different
> from what I am used to, so I am trying to break myself of those habits
> and check my work carefully, but obviously not carefully enough. Is
> there some script I can use to check for obvious things before
> checking in?
I don't know of any automated check, other than eventually having
enough people point out problems in patches to the point where you
automatically notice them on your own. That's what happened with me
anyway ;-)
>> 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.
>
> Go ahead and check it in, I have done no more than what's in the
> repository now.
Cool. I'll check in my cleanups once I confirm they compile.
>> 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?
>
> The one test case I have now is hand-made, however ghudson is working
> on a program to generate the test cases from dump files, work is just
> progressing slowly.
Cool.
> As to the 'current' file, the plan is to acquire a repository wide
> lock when updating it and when writing new revision files. It could
> be a symlink if platform portability wasn't a concern.
Even with a symlink I think we'd need some kind of locking, since I
don't think there's a way to automatically repoint a symlink in one
atomic operation. At least the last time I tried to do so I couldn't
seem to find a way to do it. If there is, I'd love to know about it
though...
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Apr 10 21:25:54 2004