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? Regards, Evgeny Kotkov
