Re: patch for cast module
От | Dave Page |
---|---|
Тема | Re: patch for cast module |
Дата | |
Msg-id | CA+OCxoyAMmG88zNTGSLjdLpomwc9JHksULQQD80xrr3b8WZLqA@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: patch for cast module (Sanket Mehta <sanket.mehta@enterprisedb.com>) |
Ответы |
Re: patch for cast module
(Sanket Mehta <sanket.mehta@enterprisedb.com>)
|
Список | pgadmin-hackers |
Thanks, patch applied. On Tue, Mar 1, 2016 at 7:20 AM, Sanket Mehta <sanket.mehta@enterprisedb.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@pgadmin.org> wrote: >> >> Thanks - committed. >> >> On Tue, Feb 23, 2016 at 1:34 PM, Sanket Mehta >> <sanket.mehta@enterprisedb.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@pgadmin.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.mehta@enterprisedb.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@pgadmin.org> wrote: >>>>>> >>>>>> And this time with the attachment... >>>>>> >>>>>> On Mon, Feb 15, 2016 at 4:53 PM, Dave Page <dpage@pgadmin.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.mehta@enterprisedb.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@pgadmin.org> >>>>>>>> wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> On Mon, Feb 15, 2016 at 8:10 AM, Sanket Mehta >>>>>>>>> <sanket.mehta@enterprisedb.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
В списке pgadmin-hackers по дате отправления:
Предыдущее
От: Dave PageДата:
Сообщение: pgAdmin 4 commit: Fix an error retrieving dependents and dependencies f
Следующее
От: Murtuza ZabuawalaДата:
Сообщение: Re: PATCH: Enhancement to backform controls [pgAdmin4]