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

Re: svn commit: r924722 - in /subversion/trunk/subversion: include/svn_wc.h libsvn_client/switch.c libsvn_client/update.c libsvn_wc/deprecated.c libsvn_wc/update_editor.c

From: Julian Foad <julian.foad_at_wandisco.com>
Date: Fri, 19 Mar 2010 00:58:43 +0000

On Thu, 2010-03-18 at 20:21 -0400, Greg Stein wrote:
> On Thu, Mar 18, 2010 at 19:54, Julian Foad <julian.foad_at_wandisco.com> wrote:
> > On Thu, 2010-03-18, Greg Stein wrote:
> >> On Thu, Mar 18, 2010 at 07:34, <julianfoad_at_apache.org> wrote:
> >> > Author: julianfoad
> >> > Date: Thu Mar 18 11:34:45 2010
> >> > New Revision: 924722
> >> >
> >> > URL: http://svn.apache.org/viewvc?rev=924722&view=rev
> >> > Log:
> >> > Clean up two revved WC API function signatures: move the wc_ctx parameter
> >> > before the first of the main input parameters, to keep those together and
> >> > to minimize the difference from the previous version. Do some doc string
> >> > clean-ups too.
> >> >
> >> > * subversion/include/svn_wc.h
> >> > (svn_wc_get_status_editor5): Fix a parameter name typo. Note doc string
> >> > out-of-dateness.
> >> > (svn_wc_crawl_revisions5): Note doc string out-of-dateness.
> >> > (svn_wc_get_update_editor4): Swap revision and wc_ctx parameters.
> >> > (svn_wc_get_switch_editor4): Swap revision and wc_ctx parameters. Refer to
> >> > svn_wc_get_update_editor4's doc string for most parameters to make it
> >> > easy for the reader to see that there are no differences.
> >>
> >> Why swap these parameters? REVISION (or TARGET_REVISION) is an OUT
> >> parameter. We always place OUT parameters first, and then the WC_CTX,
> >> then the other params.
> >
> > Bert discussed this with me on IRC too. My log message said why, but
> > actually there's more to it. I hadn't been able to discern a reason why
> > the rev-number param was moved away (relative to the previous version of
> > the API) from the other target parameters in the first place. After
> > this commit, Bert said he'd done it for the reason you state - output
> > params come before input params. But this parameter isn't an output
> > from this function, at least not in the usual sense. It's an address
> > given to this function, an address for this function to store away but
> > definitely not to read from or write to. A different function will
> > later retrieve that address and write a revision number to it.
> >
> > In that sense, the pointer is an input to this function, whereas in
> > another sense we could call it an "output pointer" meaning it's a
> > pointer that will only ever be written through (not read from), but it
> > is neither a classical "input pointer parameter" nor a classical "output
> > pointer parameter" for this function. Indeed, the code sequence in the
> > present only caller of svn_wc_get_update_editor4() depends on this
> > function call NOT overwriting the pointed-to revision number.
> >
> > I don't know of a precedent or rule for this specific situation so I
> > feel either way is OK. Bert said he was happy to call it an input
> > parameter. If you want to declare that this kind of parameter should be
> > grouped with classical output pointer parameters, I would be fine with
> > that decision and would happily change it.
>
> Conceptually, it is still an output parameter. You are saying "put the
> result HERE." That's an output parameter.

Output of the editor, not of this function, but OK, that's a fair point
of view. I'll change these two back.

r925061.

- Julian

> Sure, it takes a while to get the value; you have to drive the editor.
> But that's just timing. The concept is still an OUT param.
>
> I certainly have no problem with a redesign of the dataflow, but for
> now... I see it as an OUT parameter.
>
> (note that we have some similar-style params with some
> checksum-computing functions, too)
Received on 2010-03-19 01:59:13 CET

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.