Merge lp://staging/~wallyworld/storm/support-recursive-with into lp://staging/~launchpad-committers/storm/lp

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.

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.
>