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

Re: svn commit: r35512 - in trunk/subversion: include libsvn_client svn

From: Greg Stein <gstein_at_gmail.com>
Date: Thu, 29 Jan 2009 16:32:18 +0100

On Thu, Jan 29, 2009 at 16:14, Bert Huijben <rhuijben_at_sharpsvn.net> wrote:
>> -----Original Message-----
>> From: Bert Huijben [mailto:rhuijben_at_sharpsvn.net]
>> Sent: woensdag 28 januari 2009 17:15
>> To: 'C. Michael Pilato'; 'Bert Huijben'
>> Cc: 'Greg Stein'; dev_at_subversion.tigris.org
>> Subject: RE: svn commit: r35512 - in trunk/subversion: include
>> libsvn_client svn
>
>> Which others would you like to revert too?
>> Feel free to revert..
>>
>> svn_client_status4: r35518,r35515-r35514,r35512
>> svn_client_log5: r35242,r35245
>
> I added a non blocking note to TODO-1.6 with the 4 options of which we
> should choose one before branching 1.6:
> * revert the svn_client_status set
> * revert the svn_client_status and svn_client_log set under the assumption
> that we just create new functions later for both cases.

Ugh. I missed that one. Just as bad.

> And the last options:
> * switch to bitflags for the booleans. (I don't think we use bitflags
> anywhere, but these have the same documentation advantage).

We do, but they suck. Using a flag to create wildly variant behavior
is just nonsense. There should be *two* functions rather than one with
two behaviors.

> * move more arguments to svn_client_status_args_t to make the struct more
> usefull than just keeping 3 booleans.
>
> I would like some response in this thread from others before just choosing
> the first (status only revert) option myself. Julian and Hyrum were +1 on
> the initial ideas and Hyrum introduced svn_client_log_args_t.

Alright, but I maintain that all you're doing is making things more
complicated. You are NOT reducing the number of arguments to the
function. You're simply pushing those arguments off to an entirely
different parameter-passing mechanism. So now you have "some as
parameters" and "some buried in a struct, passed as a parameter." But
you're not reducing the total count, and certainly not simplifying.

Adding new parameters to functions (and creating new APIs) is simply
part of the evolution of the software. It is always going to happen.
Callers will always need to be aware of the new parameters -- they
were introduced *for a reason*. Shoving them off into a structure is
not going to magically make clients smarter. The structure is not
going to change the fact that clients will need to learn about the new
"parameters" and adjust their code to take advantage of them. And this
"future proofing" does nothing for *removing* parameters, like I did
in many of the svn_io.h functions.

Adding another API is not a problem. Clients can continue to use the
old APIs, so new ones have no impact on them. I don't see any stigma
or issue or other problems with adding new APIs. What *problem* are we
solving by trying to prevent svn_client_status5() and
svn_client_log6() from being introduced at some point in the future?
Why is it a problem if we add those APIs? How do we benefit by *not*
adding those APIs?

IOW, I'm seeing a "solution" for a "problem" that I don't fully understand.

You complained about the size and amount of deprecated functions. I
offered a solution for that a while back (move them to a separate
header), but people didn't like that. Said they wanted the old calls
right there next to the "proper ones to use". Maybe that idea can be
revisited. I certainly wouldn't mind culling deprecated stuff out of
the headers, shoving them all into svn_deprecated.h.

Cheers,
-g

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1069158
Received on 2009-01-29 16:32:34 CET

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