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.me...@enterprisedb.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 <dp...@pgadmin.org> wrote: > >> Thanks, patch applied. >> >> On Tue, Mar 1, 2016 at 7:20 AM, Sanket Mehta >> <sanket.me...@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 <dp...@pgadmin.org> wrote: >> >> >> >> Thanks - committed. >> >> >> >> On Tue, Feb 23, 2016 at 1:34 PM, Sanket Mehta >> >> <sanket.me...@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 <dp...@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.me...@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 <dp...@pgadmin.org> >> wrote: >> >>>>>> >> >>>>>> And this time with the attachment... >> >>>>>> >> >>>>>> On Mon, Feb 15, 2016 at 4:53 PM, Dave Page <dp...@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.me...@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 <dp...@pgadmin.org> >> >>>>>>>> wrote: >> >>>>>>>>> >> >>>>>>>>> >> >>>>>>>>> >> >>>>>>>>> On Mon, Feb 15, 2016 at 8:10 AM, Sanket Mehta >> >>>>>>>>> <sanket.me...@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 >> > >
