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

Re: [PATCH] (2nd approach) Fix issue #1976: Set arbitrary revision properties (revprops) during commit.

From: Peter Lundblad <plundblad_at_google.com>
Date: 2007-03-02 08:10:32 CET

Gerco Ballintijn writes:
> Hi,
>
> Peter Lundblad wrote:
> >
> >
> > You can read more about our compatibility rules in the hacking
> > document.
> >
>
> I did read it (several times actually). The crux is the correct and
> *complete* application of the ideas. :-)

I understand. Just wanted to make sure you knew where it was;)

>
> I am correct in thinking that *it is solvable* by copying the user
> provided editor structure, and extending it with a dummy set_edit_prop
> function, as in the following. Right?
>
It is definitely solvable. Making a wrapper editor is another way of
solving it. The only way I can think of where it matters is if a user
modifies the editor structure during the edit. The internal copy
wouldn't reflect such changes. This is contrived, I admit.
OTOH, we traditionally created wrappers in these cases.

IN any case, please make the function that gets you a compatibility editor
internal.

> A quick scan of the subversion include directory reveals the
> following affected functions, function-pointers, and structure.
>
...
> svn_ra.h
> --------
...
> svn_ra_plugin_t::get_commit_editor
> svn_ra_plugin_t::do_update
> svn_ra_plugin_t::do_switch
> svn_ra_plugin_t::do_status
> svn_ra_plugin_t::do_diff
> svn_ra_plugin_t (because of previous five)
> svn_ra_get_ra_library (because of previous)
>
Don't touch, this is compatibility crap, meaning that you don't need
svn_ra_plugin2_t etc.

> >>
> >> My intention was always to discuss and, if needed, create a v2 editor
> >> structure. I don't think its that much work. I'm not sure what you mean
> >> with a separate patch though...
> >>
> > I mean that this patch is getting big if you revise the editor as well and
> > therefore gets hard to review. A nice way of splitting it would be to revise
> > the editor in one patch and do the rest in another. The former patch is
> > useful even if the an incarnation of set_edit_or_txn_or_rev_..._prop is
> > not added.
> >
>
> I'm not sure what the purpose would be of the part without the method
> set_edit_prop. What (generic) aspect of the editor would be revised?
> (and in what way?)

Making it extensible in the future. We already have an instance that
would need it (see the new editor in
http://svn.collab.net/repos/svn/branches/fs-atomic-renames).
(And problems with getting big patches reviewed and committed is real;)

> I was thinking along the lines of properties specifing the number of lines
> changed in plain text files, the number of changed files in general, or
> some other commit or change statistic; a language-specific client that
> stores the names of the changed functions, classes, etc.; or a performance
> tracking client that stores commit durations in the server for later
> aggragation. Something like that, nothing particularly worked out.
>

OK, none of these are convincing to me. Statistics can be calculated in
advance or handled by a pre-commit hook on the server.
The performance thing is a bit contrived IMO. *I know these were just
ad hoc thoughts).

I stand with my opinion that arbitrary metadata that's available when
the commit starts (bug id, reviewer, possibly others) is the main use case we
should care about.

Best,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Mar 2 08:11:10 2007

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