Re: Problem w Favourites menu

Hi

On Wed, Mar 27, 2013 at 7:03 AM, Dinesh Kumar
<dinesh.kumar@enterprisedb.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


--
Sent via pgadmin-support mailing list (pgadmin-support@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-support