Code review comment for lp:~mbp/bzr/ui-factory

Revision history for this message
Martin Pool (mbp) wrote :

On 10 September 2010 18:43, Vincent Ladeuil <email address hidden> wrote:
>>>>>> Martin Pool <email address hidden> writes:
>
>    >> > At the moment I really dislike using **kwargs as shorthand for a dict
>    >> > to be passed to lower level code, eg to be passed in to string
>    >> > interpolating.  It makes things slightly easier on the caller it's
>    >> > true, but it also tends to break by clashing with other arguments used
>    >> > either at this level or in different implementations or as the code
>    >> > changes later on.
>    >>
>    >> I see. Well, turning the callerrs x=2, y=3 into dict(x=2, y=3) won't
>    >> such a problem. But in this case, the migration from one to the other
>    >> should occur sooner rather than later, keeping different idioms in the
>    >> code base... is painful in the long run.
>
>    > If you're talking about going back to clean up all existing cases
>    > like this, then it is a bit painful.  I think not adding any more
>    > of them is worthwhile.
>
> It will be painful *once*, but less painful for new contributors that
> won't wonder which one to use, for reviewers who won't have to say "you
> picked the wrong one", etc. Just saying ;)

Let's see if we can work out a clear expression of the code guideline.
 I don't want to ban kwargs altogether, but I think it's bad to
promise people can pass arbitrary dicts through to a lower level (eg a
format string), when the function also takes other arguments. You
should instead pass a dict.

--
Martin

« Back to merge proposal