2009/11/10 Joshua Tolley <eggyk...@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

Attachment: plperl-do-whitespace.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to