On 29.08.2014 17:26, Ivan Zhakov wrote:
>>> Please revert immediately.
>>>
>>> Proper way is to implement three specific semi-private functions in
>>> libsvn_fs_fs for collecting stats, dumping and loading FS indexes and
>>> then use in svnfsfs.
>>
>> Sounds like a good idea at the first glance, but it
>>
>> * adds a large new API (instead of moving existing declarations)
>> * requires new code and various changes to existing code
>> * may require a separate library (which would defeat the whole purpose)
>> * can be done partly or entirely at any point in the future (no guarantees
>> made that could be broken)
>>
>> This new set of API would still be fully private. Nothing for which I would
>> want to provide stability guarantees. If, at some point in the future, we
>> should provide more comprehensive and mature reporting and recovery
>> functionality, then there may be point to provide a stable API plus bindings
>> for integration into some admin environment.
>>
> Did you noticed that you moved structures without svn_fs_fs__* prefix
> to private (not internal) header? This is prohibited by our coding
> style [2].
>
> Are your going to rename fs_fs_shared_data_t and fs_fs_data_t? What
> size of changes do you expect in this case?
>
> With r1620909 every type that we're going to use in
> fs_fs_shared_data_t or fs_fs_data_t should be declared in
> svn_fs_fs_private.h, which basically means that we can end up with one
> (gigantic) svn_fs_fs_private.h containing definitions of all internal
> libsvn_fs_fs structures.
Ivan is correct. We do have certain rules for private APIs (that are not
completely internal to a single module). That said, with a bit of
judicious restructuring, I'm sure the size of the necessary change could
be kept small enough.
However, I'm more concerned about the thoughtlessness displayed by that
particular commit.
> Release branching doesn't give you reason to disrupt Subversion
> moduled design. Branching could be good reason to keep things as is,
> i.e. keeping svnfsfs as internal tool though.
We decided to make svnfsfs available by default for good reasons, but
I'm sure that no-one expected that this would involve a really hacky
exposure of FSFS internals.
We have to assume that any full committer will have no trouble following
our coding rules -- which are not arbitrary -- as a matter of course. I
admit that I was satisfied by the intent of the change in r1620909, and
never expected the actual execution would be so sloppy. I'll be very
dismayed if it turns out that we have to constantly review every line of
every commit that should by its nature be trivially correct.
As things stand, we're facing a dilemma: if we release FSFSv7 as the
default repository format in 1.9, we must have svnfsfs installed by
default, and doing that right now does not look like a simple change.
All things being equal, I'm no longer convinced that any of this is
worth the trouble it's causing in our community.
-- Brane
--
Branko Čibej | Director of Subversion
WANdisco | Realising the impossibilities of Big Data
e. brane_at_wandisco.com
Received on 2014-08-29 19:09:26 CEST