On 23/01/14 14:45, Christian Kruse wrote:
> > Well, is it context or detail? Those fields have reasonably well
> > defined meanings IMO.
>
> I find the distinction somewhat blurry and think both would be
> appropriate. But since I wasn't sure I changed to detail.
>
> > If we need errcontext_plural, let's add it, not adopt inferior
> > solutions just because that isn't there for lack of previous need.
>
> I would've added it if I would've been sure.
>
> > But having said that, I think this is indeed detail not context.
> > (I kinda wonder whether some of the stuff that's now in the primary
> > message shouldn't be pushed to errdetail as well. It looks like some
> > previous patches in this area have been lazy.)
>
> I agree, the primary message is not very well worded. On the other hand
> finding an appropriate alternative seems hard for me.
>
> > While I'm griping, this message isn't even trying to follow the
> > project's message style guidelines. Detail or context messages are
> > supposed to be complete sentence(s), with capitalization and
> punctuation to match.
>
> Hm, I hope I fixed it in this version of the patch.
>
> > Lastly, is this information that we want to be shipping to clients?
> > Perhaps from a security standpoint that's not such a wise idea, and
> > errdetail_log() is what should be used.
>
> Fixed. I added an errdetail_log_plural() for this, too.
I think you have attached wrong patch.
Thanks and Regards,
Kumar Rajeev Rastogi