Re: Information for added functionalities

Hi,

Separated in three patches:
 patch1.tar.gz - pg_scanner and frmQuery.cpp with multiple result sets support
 patch2.tar.gz - sysSettings::Flush
 patch3.tar.gz - copy/paste table(s) with flush server settings to disk

Best regards
Vladimir Kokovic
Belgrade, Serbia, 19.September 2012


On 9/18/12, Dave Page <dpage@pgadmin.org> wrote:
> Hi
>
> On Sat, Sep 15, 2012 at 6:53 AM, Vladimir Kokovic
> <vladimir.kokovic@gmail.com> wrote:
>> Hi,
>>
>> Separated in three patches:
>> patch1.tar.gz - pg_scanner and frmQuery.cpp with multiple result sets
>> support
>> patch2.tar.gz - sysSettings::Flush
>> patch3.tar.gz - copy/paste table(s)
>
> Please post the patches publicly, i.e. to the mailing list, on
> individual threads.
>
> I took a very brief look at 2 and 3, and noticed some issues you'll
> want to cleanup first:
>
> - There are whitespace-only changes which should be removed.
>
> - There are completely unrelated changes to some files - e.g. patch 3
> removes "$(srcdir)/" from the file paths in pgadmin/ui/module.mk, and
> there are changes to function argument handling.
>
> - There are still some parts of one new feature in the other - e.g.,
> patch 3 seems to contain code to flush server settings to disk.
>
> - I see very few comments explaining what code is supposed to do. This
> is not necessary if code is self explanatory, but is needed in more
> complex code blocks.
>
> I'm assuming that patch 1 has similar issues. Fixing them first will
> make the patches *much* smaller and will make it possible to review
> them.
>
> Thanks.
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>