Hi Hasini, Really appreciate your comments and thanks for spending your valuable time to do the code review. Ill be back with future implementations.
On Sat, Jan 19, 2013 at 9:38 PM, Hasini Gunasinghe <[email protected]> wrote: > I have reviewed and committed your code and design diagrams to Charon > trunk. > Also added some comments for improvements in the jira [1] itself. > Well done and thank for the contribution. > > [1] https://wso2.org/jira/browse/COMMONS-90 > > Thanks, > Hasini. > > > On Sat, Jan 19, 2013 at 12:06 PM, Dinuka Malalanayake <[email protected]>wrote: > >> Hi Hasini, >> >> Successfully done all the changes that you have mention. I have attached >> the patch in to Jira [1]. >> Is there anything need to be change please feel free to comment. >> >> [1] https://wso2.org/jira/browse/COMMONS-90 >> >> >> On Wed, Jan 16, 2013 at 11:44 PM, Dinuka Malalanayake >> <[email protected]>wrote: >> >>> Hi Hasini, >>> >>> I have changed the UML diagrams according to your comments. >>> Please refer the attached images. If there is any changes need to be >>> done feel free to comment. >>> >>> >>> On Fri, Jan 11, 2013 at 4:59 PM, Dinuka Malalanayake >>> <[email protected]>wrote: >>> >>>> Hi Hasini, >>>> >>>> Thanks for the comments. Ill be back with corrections and improvements. >>>> >>>> >>>> On Fri, Jan 11, 2013 at 4:47 PM, Hasini Gunasinghe <[email protected]>wrote: >>>> >>>>> Hi Dinuka, >>>>> >>>>> Went through the code and please find the points for improvement below: >>>>> >>>>> - Like in other endpoints, you can select the encoder, decoder >>>>> dynamically based on the content type and accept headers without hard >>>>> coding the JSON encoder and JSON decoder. >>>>> - In an improved version, bulk requests needs to be processed in a >>>>> generic manner and distinguish different operations based on "method" and >>>>> "path" in each operation. BulkRequestProcessor, BulkRequestData and >>>>> BulkResponseData in your patch could be improved accordingly. >>>>> - SCIMClient API could be improved to generate the payload in a >>>>> generic manner and use that in the sample provided in this patch. >>>>> - When improving this, we can avoid hard coded values as much as >>>>> possible. For eg: 200 OK in response in BulkResourceEndpoint >>>>> - Please use constants to refer to different values in the code. >>>>> Refer SCIMConstants and its usages. >>>>> - Please use the wso2-coding styles found at >>>>> https://sites.google.com/a/wso2.com/engineering/Home/masteringintellijidea >>>>> . >>>>> >>>>> It is good that you have done the design diagrams too. We can add them >>>>> under documentations [1]. >>>>> - You can use UML notations to show the relationships between the >>>>> classes in the class diagram >>>>> >>>>> [1] >>>>> https://svn.wso2.org/repos/wso2/trunk/commons/charon/documentation/ >>>>> >>>>> Overall It is a very good job. Hope above comments would help you to >>>>> further improve. >>>>> >>>>> Thanks, >>>>> Hasini. >>>>> >>>>> >>>>> On Tue, Jan 8, 2013 at 12:42 PM, Hasini Gunasinghe <[email protected]>wrote: >>>>> >>>>>> Great job Dinuka..! >>>>>> Appreciate a lot all your voluntarily efforts on adding bulk endpoint >>>>>> to Charon.. I think we can try to ship this with IS 4.1.0, if time >>>>>> permits. >>>>>> Will review it and get back to you with the feedback.. And once it is >>>>>> completed, we also can have a code review session when you have some free >>>>>> time.. >>>>>> >>>>>> Thanks, >>>>>> Hasini, >>>>>> >>>>>> >>>>>> On Mon, Jan 7, 2013 at 11:52 AM, Dinuka Malalanayake < >>>>>> [email protected]> wrote: >>>>>> >>>>>>> Hi All, >>>>>>> >>>>>>> I have complete the basic development of SCIM bulk endpoint. This is >>>>>>> created according to the specification [1]. Appreciate if some one can >>>>>>> do >>>>>>> the code review and give me some feedback. >>>>>>> >>>>>>> Sequence diagram and class association diagram attached here with >>>>>>> >>>>>>> As well as I have attached the patch file as a improvement in >>>>>>> Commons project [2] >>>>>>> >>>>>>> >>>>>>> [1] >>>>>>> http://www.simplecloud.info/specs/draft-scim-api-01.html#delete-resource >>>>>>> >>>>>>> [2] https://wso2.org/jira/browse/COMMONS-90 >>>>>>> >>>>>>> -- >>>>>>> Thanks, >>>>>>> Dinuka Malalanayake >>>>>>> *Software Engineer*, WSO2, Inc.; http://www.wso2.com, >>>>>>> *Linked In* : >>>>>>> http://lk.linkedin.com/pub/dinuka-malalanayake/24/438/169 >>>>>>> *Blog* : http://malalanayake.wordpress.com/ >>>>>>> *Contact* : 0772508354 , [email protected] *Skype *: >>>>>>> dinuka_malalanayake >>>>>>> >>>>>> >>>>>> >>>>> >>>> >>>> >>>> -- >>>> Thanks, >>>> Dinuka Malalanayake >>>> *Software Engineer*, WSO2, Inc.; http://www.wso2.com, >>>> *Linked In* : http://lk.linkedin.com/pub/dinuka-malalanayake/24/438/169 >>>> *Blog* : http://malalanayake.wordpress.com/ >>>> *Contact* : 0772508354 , [email protected] *Skype *: >>>> dinuka_malalanayake >>>> >>> >>> >>> >>> -- >>> Thanks, >>> Dinuka Malalanayake >>> *Software Engineer*, WSO2, Inc.; http://www.wso2.com, >>> *Linked In* : http://lk.linkedin.com/pub/dinuka-malalanayake/24/438/169 >>> *Blog* : http://malalanayake.wordpress.com/ >>> *Contact* : 0772508354 , [email protected] *Skype *: >>> dinuka_malalanayake >>> >> >> >> >> -- >> Thanks, >> Dinuka Malalanayake >> *Software Engineer*, WSO2, Inc.; http://www.wso2.com, >> *Linked In* : http://lk.linkedin.com/pub/dinuka-malalanayake/24/438/169 >> *Blog* : http://malalanayake.wordpress.com/ >> *Contact* : 0772508354 , [email protected] *Skype *: >> dinuka_malalanayake >> > > -- Thanks, Dinuka Malalanayake *Software Engineer*, WSO2, Inc.; http://www.wso2.com, *Linked In* : http://lk.linkedin.com/pub/dinuka-malalanayake/24/438/169 *Blog* : http://malalanayake.wordpress.com/ *Contact* : 0772508354 , [email protected] *Skype *: dinuka_malalanayake
_______________________________________________ Dev mailing list [email protected] http://wso2.org/cgi-bin/mailman/listinfo/dev
