On 2019/01/08 13:44, Michael Paquier wrote:
> On Tue, Jan 08, 2019 at 01:13:11PM +0900, Amit Langote wrote:
>> Yeah, I think so too. I also noticed that the patch uses
>> ERRCODE_WRONG_OBJECT_TYPE as the error code, whereas we may want to use
>> ERRCODE_FEATURE_NOT_SUPPORTED. Thoughts on that?
>
> ERRCODE_WRONG_OBJECT_TYPE is the right call I think as the feature is
> actually supported, just not for all the object types.
I meant to say that the feature to add relations to a subscription is not
supported for certain relation types, even though it's practically
*possible* to do so. But maybe, I'm misunderstanding the error code
selection policy.
>> Attached updated patch, which changes the detail message text as you
>> suggest and updates the error code.
>
> Another suggestion I would have is also to change the third message of
> CheckSubscriptionRelkind() so as its style maps the two others you are
> adding, as what's on HEAD is not a model of its kind with its
> formulation using a full sentence.
I'm not totally opposed to do that while we're here, but note that there
might be many such instances in the existing code.
Thanks,
Amit