Philip Martin <philip.martin_at_wandisco.com> writes:
> Branko Čibej <brane_at_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.kotkov_at_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
Received on 2015-06-19 16:41:49 CEST