On 19.06.2015 16:41, Evgeny Kotkov wrote: > Philip Martin <philip.mar...@wandisco.com> writes: > >> Branko Čibej <br...@wandisco.com> writes: >> >>> My main objection to this approach is that it breaks all the API >>> patterns we've ever had: it creates a function that does not return an >>> error even though it clearly fails, and relies on some notification >>> callback to report actual errors. >> We have been reporting errors via callbacks since 1.2, look at >> svn_ra_lock(). We have done it again in 1.9, look at >> svn_fs_lock_many(). >> >> Also, is the function clearly failing when it reports a verification >> error? If it is correctly diagnosing that the repository is corrupt >> then the function is in some sense succeeding. I suppose we could >> change svn_repos_notify_t.err to some other type, but svn_error_t is a >> convenient way to package the information. > +1. > > Evgeny Kotkov <evgeny.kot...@visualsvn.com> writes: > >> I sketched the other possible option with redesigning svn_repos_verify_fs3() >> API to only report errors via the notify_func(), please see the attached >> patch. Although I won't insist on going this way, I like it better, as it >> allows us to get rid of things like b->silent_errors, b->silent_running, >> juggling with standard streams and API that can yield the same error in >> two different ways. > Branko, what do you think about doing it this way? Should I work this proof- > of-concept patch up to a committable state?
Go ahead; I won't be able to for the next few days. Please remember to remove the new error code, too, if we won't be using it any more. -- Brane
