On 19.06.2015 16:41, Evgeny Kotkov wrote:
> 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?
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
Received on 2015-06-19 17:01:02 CEST