> -----Original Message-----
> From: Daniel Rall [mailto:dlr@collab.net]
> Sent: Friday, March 30, 2007 8:18 PM
> To: Paul Burba
> Cc: SVN Dev; Kamesh Jayachandran
> Subject: Re: [merge-tracking]: Don't combine repos mergeinfo
> with local mergeinfo if the later is modified.
>
> On Thu, 22 Mar 2007, Paul Burba wrote:
>
> > Dan,
> >
> > Made your recommended changes (both here and from IRC). Committed
> > r23998.
> >
> > Some comments below:
> >
> > > -----Original Message-----
> > > From: Daniel Rall [mailto:dlr@collab.net]
> > > Sent: Monday, March 19, 2007 12:59 PM
> > > To: Paul Burba
> > > Cc: SVN Dev; Kamesh Jayachandran
> > > Subject: Re: [merge-tracking]: Don't combine repos mergeinfo with
> > > local mergeinfo if the later is modified.
> > >
> > > On Mon, 19 Mar 2007, Paul Burba wrote:
> ...
> > > > +svn_error_t *svn_wc__props_modified(svn_boolean_t *modified_p,
> > > > + const char *path,
> > > > + apr_hash_t **which_props,
> > > > + svn_wc_adm_access_t
> > > *adm_access,
> > > > + apr_pool_t *pool);
> > > > +
> ...
> > > I'd rather expect svn_wc_props_modified_p() to be calling
> this new
> > > routine, rather than vice-versa. If WHICH_PROPS is NULL,
> we could
> > > bail out of the old svn_wc_props_modified_p() logic earlier.
> >
> > I created one private helper function in props.c, modified_props()
>
> Even though we stopped using svn_wc__props_modified() per
> Peter's suggestion, I'd keep it around now that it's nicely factored.
Dan,
I sheepishly admit I missed the fact that r24058 did away with the only
caller to svn_wc__props_modified() :-\ Anyway, agreed, might as well
keep it.
> ...
> > > Also, is there any overlap with svn_client__get_prop_from_wc()?
> >
> > To the extent we are getting props yes, but
> > svn_client__get_prop_from_wc() only gets one prop and
> doesn't tell us
> > if it was modified.
>
> I was wondering how much implementation these routines are
> sharing, and whether it would make sense for them to share more...
See last comment below...
> > > We call svn_client__get_prop_from_wc() from
> > > svn_client__parse_merge_info(), which is in turn called
> from a loop
> > > inside get_wc_merge_info() (meaning we're probably doing some
> > > redudant work here).
> >
> > Hmmm, maybe bulk up get_wc_merge_info() so it knows not only if
> > mergeinfo was inherited, but also if it is locally modfied?
>
> This is a moot point now, yes?
With r24058 indeed it is as far as get_wc_or_repos_merge_info() goes...
...But if you were still thinking that props.c:modifed_props() and
prop_commands.c:svn_client__get_prop_from_wc() might be able to share
some implmentation then it looks like that really comes down to
svn_wc_get_props_diffs()/svn_wc_prop_get(), where the real work of
svn_client__get_prop_from_wc() is done, using modified_props(). It
looks like there might be something doable there, though to my mind it
would result in modified_props() trying to do so much that the code
becomes unnecessarily complex and difficult to understand in pursuit of
some code consolidation. Dunno really, would welcome anyone's thoughts.
I'm kinda deep in the merge tracking elision stuff at the moment and
would like to finish that first, but will circle back to this at some
point.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Apr 2 03:39:07 2007