That makes it a whole lot more readable and digestible, thanks!
On Sun, Nov 23, 2008 at 3:04 PM, Erik Gomersbach <[EMAIL PROTECTED]> wrote: > Hi Chris, > > I've split the patch into 6 smaller patches and rebased on the current > trunk. > > Please let me know if this works for you! > > Cheers, > Erik > > ----- Original Message ----- From: "Chris Chabot" <[EMAIL PROTECTED]> > To: <[email protected]> > Sent: Sunday, November 23, 2008 12:43 PM > > Subject: Re: [jira] Commented: (SHINDIG-705) Allow extending with new > services without having to change ApiServlet code > > > agreed >> >> and please make sure they apply cleanly on the current trunk :) >> >> On Sun, Nov 23, 2008 at 12:31 PM, Erik Gomersbach <[EMAIL PROTECTED]> >> wrote: >> >> Ok, I'll split the one big patch into several smaller, more readable, >>> patches. These patches won't be independent though, you agree? >>> >>> I did update the unit tests, that's right. >>> >>> Erik >>> >>> ----- Original Message ----- From: "Chris Chabot" <[EMAIL PROTECTED]> >>> To: <[email protected]> >>> Sent: Sunday, November 23, 2008 12:22 PM >>> Subject: Re: [jira] Commented: (SHINDIG-705) Allow extending with new >>> services without having to change ApiServlet code >>> >>> >>> >>> if you could re-base the diff from the latest trunk that should already >>> >>>> save >>>> us a lot of -- and ++ on the formatting; I always prefer to have patches >>>> in >>>> the right code format, otherwise they quickly become unreadable, as we >>>> had >>>> now :) >>>> >>>> maybe if you could also split up the patches into a rest/api/data >>>> service >>>> patch, and a --/++ input/output converters, that should make that a lot >>>> easier to grok too. >>>> >>>> Sorry for being such a pain in the backside, but with a total of > 150 >>>> mil >>>> active users reached with this code, i tend to be rather-safe-then-sorry >>>> :) >>>> >>>> (ps i saw you did adjust the unit tests, right? having those in >>>> correctly >>>> is >>>> important) >>>> >>>> On Sun, Nov 23, 2008 at 12:11 PM, Erik Gomersbach <[EMAIL PROTECTED]> >>>> wrote: >>>> >>>> Hi Chris, >>>> >>>>> >>>>> thanks for having a look at this. >>>>> >>>>> I agree that the patch seems to be much larger than expected, but I'm >>>>> not >>>>> sure how easy it will be to reduce the size, but maybe you have some >>>>> idea >>>>> on >>>>> this. >>>>> >>>>> This is what the patch contains: >>>>> >>>>> The main changes are in the following files: >>>>> >>>>> RestRequestItem >>>>> ApiServlet >>>>> DataServiceServlet >>>>> >>>>> The other changes are mainly rearranging of existing code: >>>>> >>>>> The data format converters: >>>>> -InputAtomConverter >>>>> -InputJsonConverter >>>>> -InputXmlConverter >>>>> -Output AtomConverter >>>>> -OutputJsonConverter >>>>> -OutputXmlConverter >>>>> >>>>> I have replaced with data content converters: >>>>> +InputActivityConverter >>>>> +InputAppDataConverter >>>>> +InputMessagesConverter >>>>> +InputPersonConverter >>>>> +OutputStandardConverter >>>>> >>>>> Especially this causes lots of +++ and --- in the patch. >>>>> >>>>> Same holds for the updates tests. >>>>> >>>>> Please let me know how I can improve this. I am using two spaces for >>>>> indentation, so that should be in line with your latests commits ;-) >>>>> >>>>> Cheers, >>>>> Erik >>>>> >>>>> ----- Original Message ----- From: "Chris Chabot (JIRA)" < >>>>> [EMAIL PROTECTED]> >>>>> To: <[EMAIL PROTECTED]> >>>>> Sent: Sunday, November 23, 2008 11:25 AM >>>>> Subject: [jira] Commented: (SHINDIG-705) Allow extending with new >>>>> services >>>>> without having to change ApiServlet code >>>>> >>>>> >>>>> >>>>> >>>>> [ >>>>> >>>>>> >>>>>> >>>>>> https://issues.apache.org/jira/browse/SHINDIG-705?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12650011#action_12650011 >>>>>> ] >>>>>> >>>>>> Chris Chabot commented on SHINDIG-705: >>>>>> -------------------------------------- >>>>>> >>>>>> Hey Erik, >>>>>> >>>>>> First of all thanks for working on this! >>>>>> >>>>>> While trying to look through the patch, I couldn't help but noticing >>>>>> it >>>>>> --'s and ++'s a lot more then I would expect, a total of a 130k of >>>>>> changes >>>>>> is slightly more then expected :) ) (It would seem a lot of it is just >>>>>> code >>>>>> formatting stuff) >>>>>> >>>>>> To be able to actually see what this does (and not make the project's >>>>>> code >>>>>> formatting inconsistent) I would really love to have a bit more >>>>>> concise >>>>>> patch that only consists of the actual changes >>>>>> >>>>>> Allow extending with new services without having to change ApiServlet >>>>>> >>>>>> code >>>>>>> >>>>>>> >>>>>>> >>>>>>> -------------------------------------------------------------------------- >>>>>>> >>>>>>> Key: SHINDIG-705 >>>>>>> URL: https://issues.apache.org/jira/browse/SHINDIG-705 >>>>>>> Project: Shindig >>>>>>> Issue Type: Improvement >>>>>>> Components: RESTful API (PHP) >>>>>>> Reporter: Erik Gomersbach >>>>>>> Priority: Minor >>>>>>> Attachments: patch2.txt >>>>>>> >>>>>>> >>>>>>> Currently one is required to change the ApiServlet code to be able >>>>>>> the >>>>>>> extend Shindig with new services. This patch makes the extending of >>>>>>> Shindig >>>>>>> with new services configurable. >>>>>>> >>>>>>> >>>>>>> -- >>>>>> This message is automatically generated by JIRA. >>>>>> - >>>>>> You can reply to this email to add a comment to the issue online. >>>>>> >>>>>> >>>>>> >>>>>> >>>>> >>>>> >>>>> -------------------------------------------------------------------------------- >>>>> >>>>> >>>>> >>>>> No virus found in this incoming message. >>>>> Checked by AVG - http://www.avg.com >>>>> Version: 8.0.175 / Virus Database: 270.9.9/1805 - Release Date: >>>>> 11/22/2008 >>>>> 10:34 AM >>>>> >>>>> >>>>> >>>>> >>>> >>> >>> >>> -------------------------------------------------------------------------------- >>> >>> >>> >>> No virus found in this incoming message. >>> Checked by AVG - http://www.avg.com >>> Version: 8.0.175 / Virus Database: 270.9.9/1805 - Release Date: >>> 11/22/2008 >>> 10:34 AM >>> >>> >>> >> > > > -------------------------------------------------------------------------------- > > > > No virus found in this incoming message. > Checked by AVG - http://www.avg.com > Version: 8.0.175 / Virus Database: 270.9.9/1805 - Release Date: 11/22/2008 > 10:34 AM > >
