[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Get-locations - Finished (more or less)

From: Greg Hudson <ghudson_at_MIT.EDU>
Date: 2004-04-25 22:18:15 CEST

On Sun, 2004-04-25 at 03:52, Shlomi Fish wrote:
> Hmmm... is there any reason the style of the ra_svn and svnserve components is
> different than the rest of Subversion. This is highly confusing and I did not
> know what to do in this case. Future contributors may face the same problem.

I would support having a consistent style across the whole project, but
consensus seems to be against that so far. Certainly, the right answer
is not to have mixed styles within a single file.

> > This client code does not seem appropriate. An ra_lib function is not
> > supposed to take a URL argument to operate on; it is supposed to take a
> > path argument relative to the root of the RA session.
> >
> You mean the root returned by the get_repos_root() callback?

No, relative to the URL used to open the RA session.

> Well, when cmpilato and I formulated the specification for the get_locations()
> handler, we specifically said we are going to input and output URLs.

Were you guys specifically talking about the RA interface, or were you
talking about some other interface?

It seems highly inconsistent for every single ra-lib function (including
similar ones like check_path) to take a path parameter, while this one
new function takes a URL parameter. If you guys have designed this
inconsistency into the RA interface, then I think you guys have designed
it wrong, although I'm willing to listen to arguments to the contrary.

> If we are to change it now, it will require quite a lot of work. It is doable
> and I am willing to do it, but think that we should have thought about it
> earlier.

This sort of thing is going to happen. We can't make every developer
think of every problem at the earliest possible moment. So we have to
choose between making contributors go to more work, or doing things
wrong a lot. I prefer the former.

> > svn_ra_svn_write_number() would do the same thing, but consistently
> > using write_tuple() should make it easier to understand.
> >
> So what should I use?

You should use what you already have. I thought I was clear about that.

> > This is not correct C code; the representation of a void * could,
> > conceivably, be different from the representation of an svn_revnum_t *.
> Interesting. I never encountered such a situation. Oh well. If you say so.

It's not common. But it's a point of C correctness which we follow in
our code. (There are other points of C correctness we don't follow; for
instance, we use apr_pcalloc and expect it to initialize pointer
parameters to NULL, which is not valid C. I argued against this years
ago and lost.)

To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Apr 25 22:18:34 2004

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.