Re: svn commit: r1618138 - in /subversion/trunk/subversion: libsvn_fs_fs/caching.c libsvn_fs_fs/fs.c libsvn_fs_fs/fs.h libsvn_fs_fs/fs_fs.c libsvn_fs_fs/fs_fs.h libsvn_fs_fs/hotcopy.c libsvn_fs_fs/structure tests/cmdline/svnadmin_tests.py

[Branko Čibej <br...@wandisco.com>]

On 15.08.2014 11:13, kotkov@apache.org wrote:
> Author: kotkov
> Date: Fri Aug 15 10:13:23 2014
> New Revision: 1618138

May I ask why you thought it's OK to ignore my request to implement this
*major new FS feature*** on a branch and committed directly to trunk
instead? Clearly your change was incomplete, judging by the following
several commits. This is despite the fact that I plainly told you that
this community decided a while ago not to develop untested new features
on trunk. Stefan^2 even created the feature branch for you.

Now I'm going to ask you nicely to recreate that branch from current
trunk and revert this commit and all further related commits from trunk;
I believe that the full list is:

    1618138; 1618151; 1618177
but don't take my word for it. Then, when you're sure the feature is
ready, ask for a vote to get the branch merged to trunk.

Thanks,

-- Brane


-- 
Branko Čibej | Director of Subversion
WANdisco | Realising the impossibilities of Big Data
e. brane@wandisco.com

Re: svn commit: r1618138 - in /subversion/trunk/subversion: libsvn_fs_fs/caching.c libsvn_fs_fs/fs.c libsvn_fs_fs/fs.h libsvn_fs_fs/fs_fs.c libsvn_fs_fs/fs_fs.h libsvn_fs_fs/hotcopy.c libsvn_fs_fs/structure tests/cmdline/svnadmin_tests.py

[Branko Čibej <br...@wandisco.com>]

On 17.08.2014 00:53, Evgeny Kotkov wrote:
> It looks like we have another misunderstanding here with the "major FS
> feature" part. From my point of view, this is a *fix* for an existing
> FSFS feature (ability to share data and cache filesystem data),

I'm perfectly aware of what your change is for and which issues it is
intended to fix. That does not change the fact that adding another
repository identifier affects visible behaviour of FSFS, therefore it is
a major change in functionality. Furthermore, while there has been some
discussion on this topic, I have not yet seen an a list of all side
effects this new ID may have (or that we may want it to have). I
certainly do not expect the addition to only affect caching, but also
svnadmin (create, dump, load, hotcopy, ...), probably svnlook, and we
may certainly find other changes we'd want to make now that we have this
second ID.

In short, while the secondary ID in itself is a good idea, I'm missing
some discussion (e.g., in a design doc on our Wiki) about its side
effects, and an implementation of these side effects. While it's clearly
possible to make all the code changes in small steps on trunk, it's much
harder for people to review and asses the total effect than if it were
done on a branch. Yes, we often make such changes on trunk, and we
should stop doing that, especially for changes that affect the FS.

Note that the above does not imply that such a branch would have to be
maintained for the next six months, nor does it imply that your change
would not make it into 1.9. But I would like it to be (a) documented and
(b) rather more complete.

-- Brane


-- 
Branko Čibej | Director of Subversion
WANdisco | Realising the impossibilities of Big Data
e. brane@wandisco.com

Re: svn commit: r1618138 - in /subversion/trunk/subversion: libsvn_fs_fs/caching.c libsvn_fs_fs/fs.c libsvn_fs_fs/fs.h libsvn_fs_fs/fs_fs.c libsvn_fs_fs/fs_fs.h libsvn_fs_fs/hotcopy.c libsvn_fs_fs/structure tests/cmdline/svnadmin_tests.py

[Evgeny Kotkov <ev...@visualsvn.com>]

Hi Branko,

> May I ask why you thought it's OK to ignore my request to implement this
> major new FS feature on a branch and committed directly to trunk instead?
> Clearly your change was incomplete, judging by the following several
> commits. This is despite the fact that I plainly told you that this
> community decided a while ago not to develop untested new features on trunk.
> Stefan^2 even created the feature branch for you.

As I see it, this was a suggestion, not a request, and I did not ignore it.
However, the patch received proper attention, we did resolve the technical
issues with it, so I thought there is nothing wrong with committing it.

It looks like we have another misunderstanding here with the "major FS feature"
part.  From my point of view, this is a *fix* for an existing FSFS feature
(ability to share data and cache filesystem data), which does not work under
certain circumstances.  What I mean is, if this is a "major FS feature", what
would a "minor" feature look like?  And what would be a good example of a
major FS feature with the same size, impact and importance for an end user,
that went through the branch and voting procedure?

> Now I'm going to ask you nicely to recreate that branch from current trunk
> and revert this commit and all further related commits from trunk; I believe
> that the full list is:
>
>     1618138; 1618151; 1618177
>
> but don't take my word for it. Then, when you're sure the feature is ready,
> ask for a vote to get the branch merged to trunk.
>
> Thanks,
>
> -- Brane

If you insist, I will revert these changes from trunk and will attempt to commit
them as a major FS feature with branching and voting.  However, I do not really
see any technical reason to do that.  This is not a destabilizing change, and
it solves a well-known problem that even we keep stumbling upon during the
development process.


Regards,
Evgeny Kotkov