Re: [Tarantool-patches] [PATCH v4 2/3] cfg: support symbolic evaluation of replication_synchro_quorum

[Cyrill Gorcunov <gorcunov@gmail.com>]

On Sun, Dec 20, 2020 at 06:01:25PM +0100, Vladislav Shpilevoy wrote:
...
> 
> I saw the comment and I understood it. But it does not mean the function
> can now return random values. It does not matter if you return 0 or 1 or
> whatever else. It still can't be used for anything useful except check < 0.
> 
> That makes the function result confusing and even useless.
> 
> Previously quorum value was returned *and used as quorum value*. Now it
> is used only for < 0 check. Hence, why do you need to return anything
> except -1 and 0?
> 
> For example, look at box_check_replication_synchro_timeout(). It returns
> *timeout value*, which is used to set replication_synchro_timeout.
> 
> box_set_replication_synchro_quorum() before your patch did the same
> with box_check_replication_synchro_quorum(). Now it uses check() result
> only to compare it with < 0, which is *confusing* - why would it need
> to return anything but -1 and 0 then?
> 
> Here is what I want to see:

Aha, I see what you mean. I think we should split then.

 - change existing code to return 0|-1
 - then introduce new code

this allows us to see the changes more clearly