Code review comment for lp:~wallyworld/storm/support-recursive-with

Revision history for this message
Ian Booth (wallyworld) wrote :

Hi

Thank you. I'll definitely make the changes you suggest below; I think
they are all excellent points.

On 08/11/11 21:17, Free Ekanayaka wrote:
> Review: Approve
>
> Looks good to me with the 2 points below fixed. +1
>
> [1]
>
> + def __init__(self, name, select, **kwargs):
>
> Any particular reason for not writing this as:
>
> + def __init__(self, name, select, recursive=False):
>
> ?
>
> It feels more explicit to me.
>
> [2]
>
> Please document the new recursive parameter in the class docstring, like:
>
> class With(Expr):
> """Compiles to a simple (name AS expr) used in constructing WITH clauses.
>
> @param recursive:<description>
> """
>
> and add a note about it in the NEWS file.
>

« Back to merge proposal