Greg Stein <gstein@lyra.org> writes:
> I don't understand the rationale for this function.
Rationale? Is that a prerequisite for code submission? :-)
> Why do we have it, and what is the key difference from
> svn_repos_dir_delta? Can that be clarified in the documentation?
svn_repos_update exist because of the following situation:
Say I have a Greek Tree at revision 1 in my working copy. I type `svn
up A/mu'. svn_repos_dir_delta doesn't know what to do with files--it
only takes directory args. svn_repos_update, on the other hand, can
handle this.
Now, take the dir case. Let's say that someone has removed the
directory A/D/G and added a new file A/D/G. I type `svn up A/D/G.'
Once again, svn_repos_dir_delta would croak because it isn't looking
at two directories.
"So, why don't you just do the delta from one level higher," you might
be tempted to say.
"Fine," I reply, "but that means that everthing in A/D gets
updated...this is NOT what I requested."
So, what I really need is a way to say, "Mr. Update Editor Driver, I
want you to have full knowledge of the directory A/D, but I need you
promise to only pay attention to the entry G in that directory."
And svn_repos_update complies.
Perhaps I should put all that in svn_repos.h ?
> And *WHAT* is the reason for neither of them calling close_edit? Both of
> them call set_target_revision and replace_root, so why not close_edit? It is
> a pain in the butt for users of these functions to never touch the editor
> *except* to have to go and clean up after the function, using a close_edit.
> Why?
Umm...Jim? This was in the docstring for svn_repos_dir_delta well
before I re-wrote them--I have no idea what the original designer's
intent was. Personally, I'd like to see this function call
close_edit...it seems weird to have the whole editor drive
encapsulated, except (and there's the crux of the matter, that pesky
word 'except') the cleanup. As long as we have the
you-can't-call-multiple-replace-roots rule, it's not as though the
caller could actually do anything interesting with the editor between
the calls to svn_repos_update and the close_edit call.
> I created a utility variable to hold a single "/" string here, rather than
> needing to create two. Also, careful on the style (spaces).
Oops. Sorry.
> Further, this whole thing for creating an svn_string_t for a *CONSTANT* is
> *really* started to peeve me. The darned function isn't going to do any
> modification of the string, so why use an svn_string_t?
<snip svn_string_t micro-rant>
I have no complaints with an svn_string_t review occurring in the
future, but right now my primary concern is to make the stuff work.
When Subversion is in such shape that I feel I can spare more time to
debatable stylistic issues, perhaps I'll do the review myself. Anyone
else wishing to perform such a review right now is welcome to do so.
Me? I'm plugging away toward a golden one-point-oh.
Received on Sat Oct 21 14:36:29 2006