On Fri, Mar 29, 2013 at 1:17 PM, C. Michael Pilato <cmpilato_at_collab.net> wrote:
> Please note especially the section labeled "Reviewed But Need Further
> Attention". In it, I call out some of the stuff about which I couldn't come
> to clear and obvious conclusion/solution/etc. Please take a moment to
> review that section, and to follow up with an appropriate action on any API
> items which may have been the result of your own work or involve an area of
> the codebase with which you are suitably familiar.
This isn't in your reviewed but need further attention bucket but I'd
put it there:
svn_repos_authz_read2 takes "repos relative url" but such paths are
not canonical. In the client library we don't accept these and our
command line client converts these into absolute canonical URLs before
passing them into the client library. When I originally set out to
write this code I didn't really consider this. After I wrote it I
refactored it a couple of times since only handling OS filesystem
paths and URLs means you end up converting a repos relative path to a
URL only to split it back up again since to open the repo you need to
do so.
However, not doing it this way means this function ends up being weird
in that it expects path and groups_path to be canonical most of the
time but not when it's a repos relative URL.
The alternative is to take the approach we took with the client
library and force the servers to convert to a absolute URL.
While this is less efficient, after wrestling with this idea I think
it ends up being a better API.
What do other people think? It wouldn't take long to change this, but
if we don't like it later it's going to be a pain to change due to
compatibility guarantees.
Received on 2013-03-31 09:25:34 CEST