Re: [FEATURE] Add schema option to all relevant objects

From: Thom Brown <thom(at)linux(dot)com>
To: Guillaume Lelarge <guillaume(at)lelarge(dot)info>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [FEATURE] Add schema option to all relevant objects
Date: 2011-07-08 19:06:07
Message-ID: CAA-aLv4LWSB7A6vokzOJ1wub6fb6=84ePnAcgmJ=-Sjk5GmabA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

On 8 July 2011 19:46, Guillaume Lelarge <guillaume(at)lelarge(dot)info> wrote:
> On Thu, 2011-07-07 at 23:24 +0100, Thom Brown wrote:
>> On 7 July 2011 23:20, Guillaume Lelarge <guillaume(at)lelarge(dot)info> wrote:
>> > On Wed, 2011-07-06 at 11:59 +0100, Thom Brown wrote:
>> >> add_schemas_to_all_items_filtered.patch - same patch as before, but
>> >> with the above patch contents removed or adjusted to assume the fixes
>> >> have been applied.
>> >>
>> >
>> > Not yet done. I'll try to take care of this tomorrow.
>>
>> Awesomes.  Thanks Guillaume.  Please ensure I haven't sneaked anything
>> in I shouldn't have, and that I don't break anything for previous
>> versions.
>>
>
> OK, just a few things that I fixed on your patch:
>  * pgadmin/schema/pgIndex.cpp contained reversed change of one of your
>   previous patches, I don't keep that.

Good spot.

>  * all extension changes are wrong according to me because they aren't
>   schema objects, but database objects. I don't keep them.

The reason why I based it on schema is because I wanted it to inherit
the schema combobox object and its source for a list of schemas so
lots of redundant code could be removed. There should be no
functional difference, but I'm probably missing the point here. :)

>  * you added possibility to change a type, and a sequence, but forgot to
>   add the code to detect the changes in CheckChange. I fixed that

Good spot for sequence, but looking at the patch, the check is in
there for dlgType.

>  * in dlgForeignTable.cpp, you declare a direction variable, but don't
>   use it. Fixed that too.

Hehe, that wasn't mine. It was in there already. :)

>  * still in dlgForeignTable.cpp, you enable the schema widget only when
>   the user is connected in 9.1 but this guy can have this window only
>   when he's on 9.1. Fixed that.

Yes, that check didn't need to be in there.

> And that should be all. The fixed patch is attached. There is one
> remaining issue: how to refresh the object's parent node in the new
> schema?

I played around with rebuilding the node path with the new schema name
to refresh it, but I kept getting endless data type issues, and have
no idea how this node stuff works, so I'm not sure what to do about
that. Have any guidance? If not, I might have another attempt at
working out how to manipulate nodes.

Thanks for spending time on this :)

--
Thom Brown
Twitter: @darkixion
IRC (freenode): dark_ixion
Registered Linux user: #516935

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

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2011-07-08 19:15:10 Re: [FEATURE] Add schema option to all relevant objects
Previous Message Guillaume Lelarge 2011-07-08 18:46:52 Re: [FEATURE] Add schema option to all relevant objects