From: | Dave Page <dpage(at)pgadmin(dot)org> |
---|---|
To: | Dinesh Kumar <dinesh(dot)kumar(at)enterprisedb(dot)com> |
Cc: | Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>, "pgadmin-support(at)postgresql(dot)org" <pgadmin-support(at)postgresql(dot)org>, Lou Oquin <LOquin(at)nammotalley(dot)com> |
Subject: | Re: Problem w Favourites menu |
Date: | 2013-04-08 16:35:27 |
Message-ID: | CA+OCxoxyDVG1yO1=9fM_pct02F_9tZJjtZ2akCX77Ego6GM5aw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgadmin-support |
Hi
On Wed, Mar 27, 2013 at 7:03 AM, Dinesh Kumar
<dinesh(dot)kumar(at)enterprisedb(dot)com> wrote:
> Hi Dave,
>
> Thanks for the guidance on this. I am attaching the patch for the same.
Unfortunately I think there are a few issues here.
1) Please keep an eye on your formatting:
- There are spelling mistakes in comments.
- There should generally be a blank line before a comment, as they
typically introduce or describe a logical code block (see
http://wiki.postgresql.org/wiki/PgAdmin_Internals#Coding_Style, in
particular point 4)
2) You recreate the favourites array, but don't free the original
array, thus creating a memory leak.
3) The new favourites array is not synced with the UI, so although
other users changes to the files aren't overwritten, they're not shown
either. This could lead to duplicate favourites being created.
4) We probably need the same changes for Macros.
I'm not sure that fixing 3 is likely to be particularly
straightforward, and given the low impact of this issue, I'm inclined
to think your time would be better spent on other work at the moment.
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Shapiro | 2013-04-08 16:58:04 | Problem with DEBUG messages |
Previous Message | Dave Page | 2013-04-05 23:00:08 | Re: feature request - automatic SSH tunnels |