Re: patch for cast module

Hi,

I forgot tot mention bugs in previous mail.

1. System cast field was showing Yes even if the cast was not a system
cast, which I have resolved.
2. In Dependent and Dependency method in __init__.py file, unnecessary
third parameter 'cast' was being passed which I have removed

Regards,
Sanket Mehta
Sr Software engineer
Enterprisedb

On Wed, Mar 16, 2016 at 12:43 PM, Sanket Mehta <
sanket(dot)mehta(at)enterprisedb(dot)com> wrote:

> Hi Dave,
>
> PFA the patch for cast module incorporating changes regarding showing
> system objects.
> Apart from support for showing system object I have resolved few bugs in
> that, unnecessary code and added nodes.sql file.
>
> Please do review it and if it looks good, please commit.
>
>
> Regards,
> Sanket Mehta
> Sr Software engineer
> Enterprisedb
>
> On Fri, Mar 4, 2016 at 4:33 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>> Thanks, patch applied.
>>
>> On Tue, Mar 1, 2016 at 7:20 AM, Sanket Mehta
>> <sanket(dot)mehta(at)enterprisedb(dot)com> wrote:
>> > Hi,
>> >
>> > There was an error in cast module while fetching its dependency and
>> > dependent.
>> > Below is the patch which resolves this issue.
>> > Please review and commit it.
>> >
>> >
>> >
>> > Regards,
>> > Sanket Mehta
>> > Sr Software engineer
>> > Enterprisedb
>> >
>> > On Wed, Feb 24, 2016 at 10:17 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>> >>
>> >> Thanks - committed.
>> >>
>> >> On Tue, Feb 23, 2016 at 1:34 PM, Sanket Mehta
>> >> <sanket(dot)mehta(at)enterprisedb(dot)com> wrote:
>> >>>
>> >>> Hi,
>> >>>
>> >>> PFA the revised patch as per your comments.
>> >>> Please review it and let me know the feedback.
>> >>>
>> >>> Regards,
>> >>> Sanket Mehta
>> >>> Sr Software engineer
>> >>> Enterprisedb
>> >>>
>> >>> On Tue, Feb 23, 2016 at 4:10 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>> >>>>
>> >>>> Hi
>> >>>>
>> >>>> I've attached an update to this patch, in which I've done some
>> >>>> word-smithing on various comments, and adjusted the SQL templates to
>> improve
>> >>>> the formatting.
>> >>>>
>> >>>> However, it looks like it's bit-rotted, as the
>> dependents/dependencies
>> >>>> display is throwing Python errors. Please fix and then I think it's
>> just
>> >>>> about ready to commit.
>> >>>>
>> >>>> Thanks.
>> >>>>
>> >>>>
>> >>>> On Fri, Feb 19, 2016 at 11:03 AM, Sanket Mehta
>> >>>> <sanket(dot)mehta(at)enterprisedb(dot)com> wrote:
>> >>>>>
>> >>>>> Hi Dave,
>> >>>>>
>> >>>>> PFA the revise patch.
>> >>>>>
>> >>>>> It includes changes according to your review comments as well as
>> >>>>> dependency/dependent part also.
>> >>>>>
>> >>>>> Let me know in case anything is missing.
>> >>>>>
>> >>>>> Regards,
>> >>>>> Sanket Mehta
>> >>>>> Sr Software engineer
>> >>>>> Enterprisedb
>> >>>>>
>> >>>>> On Mon, Feb 15, 2016 at 10:25 PM, Dave Page <dpage(at)pgadmin(dot)org>
>> wrote:
>> >>>>>>
>> >>>>>> And this time with the attachment...
>> >>>>>>
>> >>>>>> On Mon, Feb 15, 2016 at 4:53 PM, Dave Page <dpage(at)pgadmin(dot)org>
>> wrote:
>> >>>>>>>
>> >>>>>>> That's much better. Just a couple of comments now, partly based
>> on an
>> >>>>>>> email I wrote earlier:
>> >>>>>>>
>> >>>>>>> - There is still inconsistency in comment style. Please see the
>> >>>>>>> attachment for an example. Note that there is *always* a space
>> between the
>> >>>>>>> comment marker and text.
>> >>>>>>>
>> >>>>>>> - If I try to edit a cast, I can change the description - but no
>> SQL
>> >>>>>>> is shown on the SQL tab, despite the comment being correctly
>> applied when I
>> >>>>>>> hit save. The properties pane of the main window is also not
>> updated.
>> >>>>>>>
>> >>>>>>> Otherwise, it looks fine.
>> >>>>>>>
>> >>>>>>> Thanks.
>> >>>>>>>
>> >>>>>>> On Mon, Feb 15, 2016 at 1:28 PM, Sanket Mehta
>> >>>>>>> <sanket(dot)mehta(at)enterprisedb(dot)com> wrote:
>> >>>>>>>>
>> >>>>>>>> Hi,
>> >>>>>>>>
>> >>>>>>>> PFA the revised patch with all the required comments.
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>> Regards,
>> >>>>>>>> Sanket Mehta
>> >>>>>>>> Sr Software engineer
>> >>>>>>>> Enterprisedb
>> >>>>>>>>
>> >>>>>>>> On Mon, Feb 15, 2016 at 4:18 PM, Dave Page <dpage(at)pgadmin(dot)org>
>> >>>>>>>> wrote:
>> >>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>>> On Mon, Feb 15, 2016 at 8:10 AM, Sanket Mehta
>> >>>>>>>>> <sanket(dot)mehta(at)enterprisedb(dot)com> wrote:
>> >>>>>>>>>>
>> >>>>>>>>>> Hi Dave,
>> >>>>>>>>>>
>> >>>>>>>>>> Regarding your suggestion of putting some comments in
>> javascript,
>> >>>>>>>>>> I think I have already put some comments regarding model data
>> and their
>> >>>>>>>>>> controls if any extended.
>> >>>>>>>>>>
>> >>>>>>>>>> Can you please let me know where exactly you think more
>> comments
>> >>>>>>>>>> are required?
>> >>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>>> Hi
>> >>>>>>>>>
>> >>>>>>>>> The issue for me is that jQuery code isn't the easiest to read
>> at
>> >>>>>>>>> the best of times, with nested/anonymous functions and inline
>> JSON etc. As I
>> >>>>>>>>> look through the code for the various nodes in isolation, it's
>> extremely
>> >>>>>>>>> difficult to get a sense of what exactly each part of the code
>> is doing. In
>> >>>>>>>>> this example, what I see by reading the code is:
>> >>>>>>>>>
>> >>>>>>>>> - Define the required libraries (require.js stuff)
>> >>>>>>>>> - Extend the collection class
>> >>>>>>>>> - Extend the node class
>> >>>>>>>>> - Define an init function inline
>> >>>>>>>>> - Add the menu options
>> >>>>>>>>>
>> >>>>>>>>> That part is fairly easy to figure out (easier because there are
>> >>>>>>>>> blank lines between the logical sections). From there though,
>> it becomes
>> >>>>>>>>> much harder;
>> >>>>>>>>>
>> >>>>>>>>> - There are no blank lines to separate logical code sections at
>> all
>> >>>>>>>>> between line 48 and 235 (there is one blank line, but it
>> doesn't separate
>> >>>>>>>>> code sections).
>> >>>>>>>>> - There are 4 comments that I can see. The first two are
>> identical,
>> >>>>>>>>> and appear to have identical code blocks following them for
>> reasons that are
>> >>>>>>>>> not even remotely obvious.
>> >>>>>>>>> - As a newcomer to this code, I'm wondering if it's purpose is
>> to
>> >>>>>>>>> define the backform model. If so, why is it not broken up into
>> sections with
>> >>>>>>>>> a comment to tell me what field each block handles, and any
>> other useful
>> >>>>>>>>> information I may need to know? If it's not, then what is it
>> for?
>> >>>>>>>>>
>> >>>>>>>>> So... I'm not going to tell you exactly where to put comments,
>> >>>>>>>>> because the point is that without spending a couple of hours
>> understanding
>> >>>>>>>>> this, I simply don't know. The point of the comments (and
>> separation of
>> >>>>>>>>> logical sections of code with blank lines) is to make it easy
>> for another
>> >>>>>>>>> developer (especially one as rusty as me) to read and
>> understand, then fix
>> >>>>>>>>> and improve. Be generous with comments, but don't use them
>> unnecessarily
>> >>>>>>>>> (e.g. "a = 1 // Set a to one").
>> >>>>>>>>>
>> >>>>>>>>> Of course, this is not just directed at you Sanket - it's
>> something
>> >>>>>>>>> all of us working on pgAdmin need to keep in mind.
>> >>>>>>>>>
>> >>>>>>>>> Thanks.
>> >>>>>>>>>
>> >>>>>>>>> --
>> >>>>>>>>> Dave Page
>> >>>>>>>>> Blog: http://pgsnake.blogspot.com
>> >>>>>>>>> Twitter: @pgsnake
>> >>>>>>>>>
>> >>>>>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>> >>>>>>>>> The Enterprise PostgreSQL Company
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>
>> >>>>>>>
>> >>>>>>>
>> >>>>>>> --
>> >>>>>>> Dave Page
>> >>>>>>> Blog: http://pgsnake.blogspot.com
>> >>>>>>> Twitter: @pgsnake
>> >>>>>>>
>> >>>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>> >>>>>>> The Enterprise PostgreSQL Company
>> >>>>>>
>> >>>>>>
>> >>>>>>
>> >>>>>>
>> >>>>>> --
>> >>>>>> Dave Page
>> >>>>>> Blog: http://pgsnake.blogspot.com
>> >>>>>> Twitter: @pgsnake
>> >>>>>>
>> >>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>> >>>>>> The Enterprise PostgreSQL Company
>> >>>>>
>> >>>>>
>> >>>>
>> >>>>
>> >>>>
>> >>>> --
>> >>>> Dave Page
>> >>>> Blog: http://pgsnake.blogspot.com
>> >>>> Twitter: @pgsnake
>> >>>>
>> >>>> EnterpriseDB UK: http://www.enterprisedb.com
>> >>>> The Enterprise PostgreSQL Company
>> >>>
>> >>>
>> >>
>> >>
>> >>
>> >> --
>> >> Dave Page
>> >> Blog: http://pgsnake.blogspot.com
>> >> Twitter: @pgsnake
>> >>
>> >> EnterpriseDB UK: http://www.enterprisedb.com
>> >> The Enterprise PostgreSQL Company
>> >
>> >
>>
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
>