Re: Information for added functionalities

Hi

As I mentioned below, there are issues with the patches that need to
be cleaned up before they can be removed. A sizeable percentage of
patch3 for example changes things that are completely unrelated to
what the patch claims to do, or just changes paths for unrelated files
in the makefiles.

Please clean that up, and then post the patches *separate* email
threads. Noone can review them as they are now.

Thanks.

On Wed, Sep 19, 2012 at 3:46 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) 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
>>
>
>
> --
> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgadmin-hackers
>



--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company