On Sun, Feb 23, 2014 at 11:33 PM, Stefan Fuhrmann < stefan.fuhrm...@wandisco.com> wrote:
> On Fri, Feb 21, 2014 at 6:01 PM, Julian Foad > <julianf...@btopenworld.com>wrote: > >> Stefan Fuhrmann wrote: >> > Julian Foad wrote: >> >> Stefan Fuhrmann wrote: >>> >>> >> or something very much like that. Those assumptions look like the >>> real problem here. >>> >> >>> >> >>> >>> I see the following options: >>> >>> >>> >>> * Redefine svn_fs_contents_changed and svn_fs_props_changed to match >>> >>> the current implementation. I'd rather not but that just a -0 from >>> my side. >>> >> >>> >> We should deprecate them, and adjust their doc strings with a >>> historical note >>> >> about the discrepancy, and replace them with one or two new (pairs >>> of) APIs: >>> >> >>> >> -- a quick optimization API -- the definitely/maybe question -- like >>> the >>> >> existing implementations but documented properly and named >>> appropriately; and >>> >> >>> >> -- (perhaps) a definite content comparison API: "has the content >>> changed?" >>> > >>> > I tend to prefer an extra flag parameter in a bumped version of the two >>> > API functions as it makes existing API users aware of how the API has >>> > been changed. >>> >>> Well, this is just a general API design style issue, but I think a name >>> change is equally visible. I think a boolean parameter is appropriate where >>> the caller is likely to choose from both variants at run time, but in this >>> case I think callers will choose one or the other at design time, never at >>> run time. I think we have far too many inappropriate boolean behaviour >>> modifiers already in Subversion APIs. I think they make the call sites hard >>> to read. >>> >> >> Fair point. I was thinking about revving svn_ra_get_file_revs2 > as well with the same new option (callers can chose whether > need exact reports or want to process the delta anyway). > That would be a pass-through value for svn_fs_*_changed. > I implemented just that in my working copy and it turned out to be a mess. Every RA layer had to be updated and in the end, there would only be the blame function to use it - with some fixed value for the STRICT option. So, I decided to go with the separate API functions for the different behaviors in r1573111. -- Stefan^2.
