We have a process to follow up these stuff - Creating JIRAs is part of that. Please refer the "code review guidelines" doc I shared yesterday.
Thanks, Hiranya On Wed, Apr 18, 2012 at 5:09 PM, Afkham Azeez <az...@wso2.com> wrote: > Have these notes been added to Crucible? Also, we need to create one or > more corresponding Jira issues to keep track of the improvements. In the > past, we lost track of many of the comments & suggestions because there was > no way to follow up. If there are Jira issues linked to the relevant > Crucible projects, that will not happen. > > On Wed, Apr 18, 2012 at 4:54 PM, Suresh Attanayaka <sur...@wso2.com>wrote: > >> Hi All, >> >> Following are some of the points and suggestions brought up at the code >> review, please feel free to add up the missing points. >> >> >> - Rename the modules. The name "Deployment" doesn't give the correct >> meaning, suggestion was to rename it to SCIM Impl. >> - Remove the invalid annotations in the source. >> - Instead of using a single validator for SCIM object validation, use >> multiple validators for each type. Use an abstract validator and then >> create concrete validators for each type. >> - Extract a private method from the attribute encoder methods >> (simple, complex etc) and move all the common stuff to over there. >> - Write test cases for all the Parsers. >> - In the createUser() method, do all the sanity checkups at the >> beginning of the method. >> - At the getEncoder() method of the AbstractResourceEndpoint, the >> method should throw an exception if the encodeMap object is empty. >> - Do log the code. Use commons logging. >> - Warnings must be used at the removeAnyReadOnlyAttributes() method. >> - Try to optimize the nested if statements in >> the removeAnyReadOnlyAttributes() method. >> - Move encodeSCIMException() method to another Utill class instead of >> having it in an Abstract class. >> - Improve the Java Docs. Mention when exceptions are thrown etc. >> - Try to reuse HTTP constants instead of redefining them if possible. >> - The length check is unnecessary in the setEmails() method. >> - Write test cases for the basics of the SCIM protocol. >> >> Thanks & regards, >> -Suresh >> >> On Tue, Apr 17, 2012 at 7:21 PM, Hasini Gunasinghe <has...@wso2.com>wrote: >> >>> more details >>> »<https://www.google.com/calendar/event?action=VIEW&eid=bDdsZHBlbGUwNmk0cDhsOXQ1ZDJnZDNrMTAgc3VyZXNoQHdzbzIuY29t&tok=MTUjaGFzaW5pQHdzbzIuY29tZTNhZDYyZGE3ZjAyOGEwYTEyNThlNWZkZjI1Nzg4NWU0MzViODEwMQ&ctz=Asia/Colombo&hl=en> >>> [Integration TG] Code Review >>> Details: >>> Project being reviewed: WSO2 Charon >>> Link to crucible >>> project:http://wso2.org/crucible/cru/WSCC001-1<http://www.google.com/url?q=http%3A%2F%2Fwso2.org%2Fcrucible%2Fcru%2FWSCC001-1&ust=1334677897020000&usg=AFQjCNE5LnfBOQT6lBrlICwSfaHDdPVX4A> >>> Other info - Link to the related class diagrams: >>> https://svn.wso2.org/repos/wso2/trunk/commons/charon/documentation/<http://www.google.com/url?q=https%3A%2F%2Fsvn.wso2.org%2Frepos%2Fwso2%2Ftrunk%2Fcommons%2Fcharon%2Fdocumentation%2F&ust=1334677897020000&usg=AFQjCNG5MSVi2fM-Ho-hC3VJUN1UXUsfoQ> >>> *When* >>> Wed Apr 18 10am – 11am Colombo >>> *Where* >>> LK #58 5th Floor - Meeting room >>> (map<http://maps.google.lk/maps?q=LK+%2358+5th+Floor+-+Meeting+room&hl=en> >>> ) >>> *Calendar* >>> sur...@wso2.com >>> *Who* >>> • >>> has...@wso2.com - organizer >>> • >>> Asela Pathberiya >>> • >>> Johann Nallathamby >>> • >>> Thilina Buddhika >>> • >>> Hiranya Jayathilaka >>> • >>> dev@wso2.org >>> • >>> Suresh Attanayaka >>> >>> Going? >>> ***Yes<https://www.google.com/calendar/event?action=RESPOND&eid=bDdsZHBlbGUwNmk0cDhsOXQ1ZDJnZDNrMTAgc3VyZXNoQHdzbzIuY29t&rst=1&tok=MTUjaGFzaW5pQHdzbzIuY29tZTNhZDYyZGE3ZjAyOGEwYTEyNThlNWZkZjI1Nzg4NWU0MzViODEwMQ&ctz=Asia/Colombo&hl=en>- >>> Maybe<https://www.google.com/calendar/event?action=RESPOND&eid=bDdsZHBlbGUwNmk0cDhsOXQ1ZDJnZDNrMTAgc3VyZXNoQHdzbzIuY29t&rst=3&tok=MTUjaGFzaW5pQHdzbzIuY29tZTNhZDYyZGE3ZjAyOGEwYTEyNThlNWZkZjI1Nzg4NWU0MzViODEwMQ&ctz=Asia/Colombo&hl=en>- >>> No<https://www.google.com/calendar/event?action=RESPOND&eid=bDdsZHBlbGUwNmk0cDhsOXQ1ZDJnZDNrMTAgc3VyZXNoQHdzbzIuY29t&rst=2&tok=MTUjaGFzaW5pQHdzbzIuY29tZTNhZDYyZGE3ZjAyOGEwYTEyNThlNWZkZjI1Nzg4NWU0MzViODEwMQ&ctz=Asia/Colombo&hl=en> >>> * **more options >>> »<https://www.google.com/calendar/event?action=VIEW&eid=bDdsZHBlbGUwNmk0cDhsOXQ1ZDJnZDNrMTAgc3VyZXNoQHdzbzIuY29t&tok=MTUjaGFzaW5pQHdzbzIuY29tZTNhZDYyZGE3ZjAyOGEwYTEyNThlNWZkZjI1Nzg4NWU0MzViODEwMQ&ctz=Asia/Colombo&hl=en> >>> >>> Invitation from Google Calendar <https://www.google.com/calendar/> >>> >>> You are receiving this email at the account sur...@wso2.com because you >>> are subscribed for invitations on calendar sur...@wso2.com. >>> >>> To stop receiving these notifications, please log in to >>> https://www.google.com/calendar/ and change your notification settings >>> for this calendar. >>> >> >> >> >> -- >> Suresh Attanayake >> Software Engineer; WSO2 Inc. http://wso2.com/ >> Blog : http://sureshatt.blogspot.com/ >> Twitter : https://twitter.com/sureshatt >> LinkedIn : http://lk.linkedin.com/in/sureshatt >> Mobile : +94755012060,+94770419136,+94710467976 >> >> >> _______________________________________________ >> Dev mailing list >> Dev@wso2.org >> http://wso2.org/cgi-bin/mailman/listinfo/dev >> >> > > > -- > *Afkham Azeez* > Director of Architecture; WSO2, Inc.; http://wso2.com > Member; Apache Software Foundation; http://www.apache.org/ > * <http://www.apache.org/>** > email: **az...@wso2.com* <az...@wso2.com>* cell: +94 77 3320919 > blog: **http://blog.afkham.org* <http://blog.afkham.org>* > twitter: **http://twitter.com/afkham_azeez*<http://twitter.com/afkham_azeez> > * > linked-in: **http://lk.linkedin.com/in/afkhamazeez* > * > * > *Lean . Enterprise . Middleware* > > -- Hiranya Jayathilaka Associate Technical Lead; WSO2 Inc.; http://wso2.org E-mail: hira...@wso2.com; Mobile: +94 77 633 3491 Blog: http://techfeast-hiranya.blogspot.com
_______________________________________________ Dev mailing list Dev@wso2.org http://wso2.org/cgi-bin/mailman/listinfo/dev
