Re: patch for cast module

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