Re: pgAdmin Event Trigger Compatibility - Mailing list pgadmin-hackers
Hi Dinesh
I have reviewed your event trigger patch. Following is my review comments:
- There is some problem in "refresh" logic. When I clicked on newly created event trigger node, it gets collapsed automatically and the parent node gets selected.
- According to the logic you have disabled the "Default Value" text box in "New Trigger Function..." dialog, but when we select the "IN" radio button on "Parameter" tab it gets enabled.
- There should be a status bar in "New Event Trigger" dialog. With current implementation user will not know about which inputs are needed/remaining to enable the "OK" button.
- In pgFunction.cpp why we are checking the typname as "\"trigger\"" and "trigger" if these the special case then should we need it for event_trigger as well? Below is the code syntax
- "else if (typname == wxT("\"trigger\"") || typname == wxT("trigger") || typname == wxT("event_trigger"))"
- On "New Event Trigger" dialog help is not working. When we clicked on Help button it will show "The URL you specified does not exist" in web browser.
On Mon, Jul 22, 2013 at 6:39 PM, Dinesh Kumar <dinesh.kumar@enterprisedb.com> wrote:
Hi Akshay,Thank you so much for correcting me on this. It's my bad to forgot the naming convention in the code. Now, ::OnOK() is get populating the required SQL as per the changes.Please find the new patch and let me know your valuable inputs.Dinesh
--
Dinesh KumarSoftware Engineer
Ph: +918087463317Skype ID: dinesh.kumar432www.enterprisedb.com
Follow us on Twitter
@EnterpriseDB
Visit EnterpriseDB for tutorials, webinars, whitepapers and moreOn Mon, Jul 22, 2013 at 5:58 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:Hi DineshI have just started reviewing your code and found on issue where any changes made on Event Trigger dialog won't be saved. After looking at the code, you haven't override the ::OnOK() function in dlgEventTrigger.cpp. Can you please fix this and resend the new patch.--On Wed, Jul 10, 2013 at 9:46 PM, Dinesh Kumar <dinesh.kumar@enterprisedb.com> wrote:Yes, we can change this enable check box as a radio button. But, "REPLICA/ALWAYS" are two enable's properties. Hence, We have implemented this in the proposed way. Kindly share your opinion on this.So: "Enabled Replica ( ) Enabled Always ( ) Disabled ( )" ?Yes Dave.Dinesh
--
Dinesh KumarSoftware EngineerSkype ID: dinesh.kumar432www.enterprisedb.com
Follow us on Twitter
@EnterpriseDB
Visit EnterpriseDB for tutorials, webinars, whitepapers and moreAkshay Joshi
Senior Software Engineer
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
Phone: +91 20-3058-9522
Mobile: +91 976-788-8246
.jpg){kind=logo}
Akshay Joshi
Senior Software Engineer
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
Phone: +91 20-3058-9522
Mobile: +91 976-788-8246
Senior Software Engineer
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
Phone: +91 20-3058-9522
Mobile: +91 976-788-8246
pgadmin-hackers by date: