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
