> ------- Additional Comments From firstname.lastname@example.org 2003-04-16 11:46 PDT -------
> Noted blocking issue.
> Ben: have you folded in all of my commentary? It seems that some items
> (e.g. baton simplification) weren't put in. Do you have changes
> pending that I should wait on, or should I review again on the dev list?
I folded in almost all of your commentary; see r5620.
The things I folded in:
* dir_baton doesn't need the "props" member
* it doesn't look like you use hb->source or hb->dest
* in apply_textdelta(), you should use svn_stream_empty()
rather than passing hb->source to stream_from_aprfile
* why handler_pool instead of just 'pool'
* dir_baton doesn't need the parent_dir_baton. thus, it only holds
the edit_baton. thus, just return the edit_baton as a dir_baton
* hb doesn't need fb. just store tmppath
* no need to init tmp_file in apply_textdelta
* shouldn't it return something like "obstructed update"
if it finds something already there?
* hmm. the svn_io_check_path() shouldn't even be needed in
add_directory. nor in add_file. as long as you assume the root dir
cannot exist, then the other checks are useless. if we allow the
root dir to exist, then we only need to check for the existence (not
the type) of the other items. if it exists, toss "obstructed export"
There are two things I didn't agree with or understand, though:
* the error checkingin window_handler can be simplified. no need for
the first "if"
--> huh? we need that 'if' to discover whether it was a NULL
--> window or whether we got a real error.
* why keep a full hash of the props when you're only interested in
three of them.
--> actually, we're about to use a whole bunch of the
--> svn:wc:entry: props as keyword substitution values. I'd
--> rather not bother to filter the proplist just yet. maybe
To unsubscribe, e-mail: email@example.com
For additional commands, e-mail: firstname.lastname@example.org
Received on Wed Apr 16 20:56:52 2003