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
