2009/11/10 Joshua Tolley <eggyknap@gmail.com>:
> Ok, updated patch attached. As far as I know, this completes all outstanding
> issues:
>
Hi Joshua,
I'm taking a look at this patch for the commitfest. I see that Andrew
has already taken an interest in the technical aspects of the patch,
so I'll focus on submission/code style/documentation.
I noticed that there was a fairly large amount of bogus/inconsistent
whitespace in the patch, particularly in the body of
plperl_inline_handler(). Some of the lines were indented with tabs,
others with spaces. You should stick with tabs. There were also a
lot of lines with a whole lot of trailing whitespace at the end.
See attached patch which repairs the whitespace. I see you generated
the patch with git, so I recommend `git diff --check`, it'll helpfully
report about some types of whitespace error.
In the documentation you refer to this feature as "inline functions".
I think this might be mixing up the terminology ... although the code
refers to "inline handlers" internally, the word "inline" doesn't
appear in the user-facing documentation for the DO command. Instead
they are referred to as "anonymous code blocks". I think it would
improve consistency if the PL/Perl mention used the same term.
Apart from those minor quibbles, the patch appears to apply, compile
and test fine, and work as advertised.
Cheers,
BJ
