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. br...@wandisco.com
