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
_______________________________________________ Dev mailing list [email protected] http://wso2.org/cgi-bin/mailman/listinfo/dev
