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