On Thu, Aug 28, 2008 at 1:41 AM, Ian Boston <[EMAIL PROTECTED]> wrote: > The concept looks good but there is something funny going on with > ResponseItem in the patch > > > Index: > java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/ResponseItem.java > =================================================================== > --- > java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/ResponseItem.java > (revision 689519) > +++ > java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/ResponseItem.java > (working copy) > @@ -15,28 +15,31 @@ > * KIND, either express or implied. See the License for the > * specific language governing permissions and limitations under the > License. > */ > -package org.apache.shindig.social; > +package org.apache.shindig.social.opensocial.service; > > +import com.google.common.base.Objects; > + > +import org.apache.shindig.social.ResponseError; > + > > > This looks like a move and then a diff on the result of the move ?
Yes, that's what the patch is trying to do. > Should this be preceded by a mv > java/social-api/src/main/java/org/apache/shindig/social/ResponseItem.java > java/social-api/src/main/java/org/apache/shindig/social/opensocial/service/ResponseItem.java > ? Or followed by it, since ResponseItem doesn't really belong in opensocial/service until this patch is in. Would you rather we keep ResponseItem where it is for this patch, and move it in a second patch? -- Adam > On 28 Aug 2008, at 01:09, Adam Winer (JIRA) wrote: > >> >> [ >> https://issues.apache.org/jira/browse/SHINDIG-549?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel >> ] >> >> Adam Winer updated SHINDIG-549: >> ------------------------------- >> >> Attachment: shindig-549.patch >> >> Here's a first pass at implementing this, off of svn revision 689672. All >> Shindig tests pass, though I'd like a bit more manual testing. >> >>> Use exceptions instead of ResponseItem to signal service errors >>> --------------------------------------------------------------- >>> >>> Key: SHINDIG-549 >>> URL: https://issues.apache.org/jira/browse/SHINDIG-549 >>> Project: Shindig >>> Issue Type: Improvement >>> Components: RESTful API (Java) >>> Reporter: Adam Winer >>> Attachments: shindig-549.patch >>> >>> Original Estimate: 24h >>> Remaining Estimate: 24h >>> >>> As discussed on shindig-dev: >>> (1) Get error code and message out of >>> RestfulItem/RestfulCollection/DataCollection and into an exception, >>> so: >>> PersonService { >>> Future<RestfulCollection<Person>> getPeople(...); >>> Future<RestfulItem<Person>> getPerson(...); >>> } >>> becomes: >>> PersonService { >>> // Note: the Future may also throw a SocialApiException (wrapped in >>> an EvaluationException) >>> Future<RestfulCollection<Person>> getPeople(...) throws >>> SocialApiException >>> Future<Person> getPerson(...) throws SocialApiException >>> } >>> RestfulItem goes away entirely, and RestfulCollection doesn't extend >>> ResponseItem. ResponseItem can entirely disappear from the SPI, and >>> can move out of org.apache.shindig.social and into >>> o.a.s.s.opensocial.service alongside of RequestItem. >>> One advantage of this change is that it makes it easy to write generic >>> preconditions across datatypes, and gets rid of early returns from >>> service implementations, which are a bit of a code smell. >> >> -- >> This message is automatically generated by JIRA. >> - >> You can reply to this email to add a comment to the issue online. >> > >
