Line 603 | 611 | 624 | 639 | 666 : We should show a check for errors
prior to investigating result.[value]. People cut and paste this code
all the time. Error checking shouldn't be confusing in the spec. It
could be as simple as adding
if (!result.error){/*existing code*/}
Functions section (starting at 676): Each method that takes optional
parameters should have an <xref> to where the optional parameters are
defined. Also, do any examples show how to pass in the optional
parameters (I didn't see any)? If not, please add examples.
Line 680: "Takes an optional. set" should be "Takes an optional set"
(delete the period after optional)
From: opensocial-and-gadgets-s...@googlegroups.com
[mailto:opensocial-and-gadgets-s...@googlegroups.com] On Behalf Of Lane
LiaBraaten
Sent: Friday, April 03, 2009 1:16 PM
To: opensocial-and-gadgets-s...@googlegroups.com
Cc: shindig-dev@incubator.apache.org
Subject: [opensocial-and-gadgets-spec] Re: OS Lite (OSAPI) spec doc
Here's the next pass: http://codereview.appspot.com/33058
On Fri, Apr 3, 2009 at 1:04 PM, Scott Seely <sse...@myspace-inc.com>
wrote:
Yes to the eref.
I'm a bit concerned about the lack of parity between the JSON-RPC and
osapi for album, mediaitem, and messaging. Osapi and JSON-RPC are
closely related.
I'm fine with leaving these undefined until v.next. Please post when you
are complete and I'll give this another pass.
From: opensocial-and-gadgets-s...@googlegroups.com
[mailto:opensocial-and-gadgets-s...@googlegroups.com] On Behalf Of Lane
LiaBraaten
Sent: Friday, April 03, 2009 11:52 AM
To: opensocial-and-gadgets-s...@googlegroups.com
Cc: shindig-dev@incubator.apache.org
Subject: [opensocial-and-gadgets-spec] Re: OS Lite (OSAPI) spec doc
Thanks for the review, Scott. I synced up with Bob to address the open
issues on this thread. I'm working on an updated patch, but in the mean
time, comments inline...
-Lane
On Fri, Apr 3, 2009 at 9:59 AM, Scott Seely <sse...@myspace-inc.com>
wrote:
Based on my review, this is still a work in progress. I do not guarantee
that this is a complete list of spec issues. These are simply the issues
that seem to be the most evident.
Missing from spec:
Messaging
Albums
MediaItem
I think we should move these to v.next
Line 588: "Highlights" is really a section. Please change from
<t> to <section title="Highlights">
fixed
Line 600: Please strike. This is a sentence that occurs in the
discussion of why you included the examples early. It is not needed for
the spec.
removed
Line 601: "A Person request, the most basic example" should be
moved into the <figure> tag as:
<figure>
<preamble>A basic Person
Request</preamble>
Line 609 & 618: Move content inside /figure/preamble tag
fixed here and in a couple other places as well
Line 631 Move this to Highlights list: Batch calls can be
cascaded.
Maybe I'm old school, but it seems that the correct term is
"chained|chaining"? Suggested edit when moving to Highlights (near line
588)
"Batch calls are designed for call chaining."
The examples demonstrate this concept adequately.
Moved to highlights list and changed to 'chaining'.
Line 635. Please strike "This is a short namespace, so it's
easier to type, and it is different from the existing apis so that they
can coexist until the old apis are removed." I don't believe that this
statement enhances the spec. Line 586 already addresses deprecation.
removed
Line 648: "When the Gadget server renders the osapi library, it
calls the SystemHandler's list Methods endpoint, and adds the available
services."
'it' is ambiguous.
SystemHandler is a new term. I believe that this may be a
Shindig type, not something that is actually part of the system.
Line 648 contains a TODO: "The js client then uses this to
generate (TODO Not implemented yet)". The document here MUST NOT refer
to TODOs in any given implementation. The document MUST NOT contain
TODOs for feature descriptions. I'm not sure which case the TODO refers
to, but the TODO needs to disappear and be rectified.
removed implementation details (and TODO). The paragraph now reads:
The features available on the client are determined by the server from
which the JavaScript is delivered. The server SHOULD generate service
methods for all available services. Developers wanting to write gadgets
safely can test for the existence of a service and method before making
their call.
[example follows...]
Line 661: Define behavior of <Require feature="osapi" />
Line 662 | 682 | 698 | 715 | 735 | 750 | 775
Why are we defining subsets of the feature?
Osapi should be a single feature, introducing the basic APIs around
Activities, People, AppData. The extra granularity seems to be new and
unnecessary. I fail to see the benefit of the extra knobs.
Let's just use feature="osapi" for v0.9. We can revisit sub-features
later, but at this point it doesn't seem to provide much benefit.
Line 662: "that map to the activities endpoint" should be "that
map to the people endpoint". Need precise definition of what the
endpoint is. Probably need a cross reference to the spec where the
endpoint for each item is defined
will do...this is an eref tag, right?
Line 666 | 667 | 668 | 669: You repeat "convenience" 2x. Delete
the 'Note'
removed the duplication
Line 682 | 698: Need precise definition of what the endpoint is.
Probably need a cross reference to the spec where the endpoint for each
item is defined
will do.
From: opensocial-and-gadgets-s...@googlegroups.com
[mailto:opensocial-and-gadgets-s...@googlegroups.com] On Behalf Of Lane
LiaBraaten
Sent: Thursday, April 02, 2009 9:45 PM
To: opensocial-and-gadgets-s...@googlegroups.com
Cc: shindig-dev@incubator.apache.org
Subject: [opensocial-and-gadgets-spec] Re: OS Lite (OSAPI) spec
doc
oops - here's the patch: http://codereview.appspot.com/33058
On Thu, Apr 2, 2009 at 9:42 PM, Lane LiaBraaten
<lliab...@google.com> wrote:
I created a spec patch for the OSAPI stuff, but I didn't realize
Bob hadn't incorporated Adam's comments yet. I made a few changes, but
some of these we still need to finalize. My comments inline.
-Lane
On Tue, Mar 31, 2009 at 10:10 AM, Adam Winer <awi...@gmail.com>
wrote:
Comments:
- At this stage, we really need spec updates in the form
of diffs to
http://opensocial-resources.googlecode.com/svn/spec/draft/Opensocial-Spe
cification.xml.
I kinda wish we'd split this huuuge doc into multiple
smaller docs,
as the XSLT transform hangs Firefox for awhile, but that
should be its
own spec diff.
- osapi.base should largely be undocumented, and I'm not
sure it
should exist as a feature name, though I could be
convinced otherwise.
no change ... I'm not clear on this so I don't have an opinion
right now
I'm going to take osapi.base out of the spec for v0.9
- if (osapi.people.getViewer) {
should be
if (osapi.people && osapi.people.getViewer) {
since you have to test for both the service and the
method.
fixed
- "When no parameters are specified, the server defaults
will be used:
{ user : @me, group : @self}. This is built on
osapi.newJsonRequest()."
Two notes. It's userId: @me, groupId: @self. And
what it's built
on is an implementation detail that the spec should
leave out.
fixed
- osapi.people.get.execute(callback)
isn't a great description, since there is no such
function on
osapi.people.get. Need some central documentation
describing the
execute() method on all request and batch objects
no change - not sure what the change should be here
will add central documentation about "Request objects" (which have
execute() methods) and "Batch Request objects" (which have execute() and
add() methods)
- osapi.activities.create(params) - maps to the
activities create
endpoint. This is a non-ui intermediated version of the
osapi.ui.requestCreateActivity call.
I think the distinction between UI-intermediated calls
and
non-UI-intermediated calls is artificial. A container
may make *any*
JS call ui-mediated - permission dialogs being the
obvious example.
Strongly prefer if we:
(1) Kill requestCreateActivity(), and for that matter
everything in osapi.ui
(2) Leave UI mediation up to the container, in
particular use
activities.create() instead of requestCreateActivity()
(3) If we have time, build the message service API for
the
requestSendMessage()/requestShareApp(), if not leave
that for v.Next
(4) Don't bother with requestPermission() in this API,
though that
opinion is 99% based on experience with a few
containers, and other
container developers may have very different feedback.
no change, but I'm +1 on these comments
goodbye osapi.ui
- osapi.appdata.deleteData
Since JS prevents us from just calling this "delete",
I'd rather
call it "remove()", which is at least a predictable
naming pattern.
Or "deleteAppdata()".
no change, but I'm +1
Bob thinks he can get delete() to work...if not, we'll use remove()
- " osapi.messages TODO"
I think this means it's not in 0.9.
removed
* osapi.newBatch() - returns a new osapi.base.batch
request.
* osapi.base.batch.add(key, request) - Adds a service
request to
the batch associated with the specified key. These may
be jsonrpc
calls, such as osapi.people.get(), or osapi.makeRequest
calls to third
parties, or a mixture of the two.
* osapi.base.batch.execute(callback) - executes all
of the
requests in the batch. Callback is passed a JSON object
mapping each
request key to a JSON response object.
Don't doc that batches are in "osapi.base.batch". All
we need to doc
is that batch objects have add() and execute() methods.
no change, I think this makes sense though, esp if we take out
the osapi.base docs
see commnet above about documenting "Batch Request objects"
- Batch functions:"
One thing I would recommend adding is tweaking
osapi.newBatch() to be:
osapi.newBatch(opt_json)
... where opt_json is a JSON array describing a full
request. This
lets developers that are truly fluent with JSON-RPC
write:
osapi.newBatch([{method: "people.get"} ...
etc...]).execute(callback);
... and done.
no change, seems reasonable
Let's punt on this for now...I don't think many developers will use it
- osapi.makeRequest
This should be consistent with everything else in
osapi, not with
gadgets.io. I think the right API is to follow Data
Pipelining:
osapi.http.get(params)
osapi.http.post(params)
(and any other HTTP methods we care to specify at this
point).
"params" should exactly match the attributes on
os:HttpRequest, and
must use the same data format returned by os:HttpRequest
(which I
haven't specified yet, I will send out that today). It
also shouldn't
mention that it uses gadgets.io.makeRequest().
no change, but I like it
I'm planning to create methods for head, get, put, post, delete
- osapi.ui:
As said above, I think this needs to be killed, as
ui-mediated is up
to the container, not the spec. Batching semantics get
a little
tricky, but we're far better off supporting batching for
ui-mediated
requests than not, because it gives the container a lot
more
flexibility for good UIs - instead of showing 5 dialogs
in succession,
the container can show a single dialog saying the
gadget's trying to
do 5 things. For now, we can leave the batch semantics
up to the
container, as I can come up with a variety of plausible
semantics.
Also, everything else is
method(params).execute(callback), and there's
no reason to break down and use positional parameters
here.
no change - the ui namespace does seem strange since there's
nothing that says the container *has* to make these calls user-mediated
I think we can get rid of the ui namespace. I agree that we should
operate under the assumption that any js call can be ui-mediated.
- osapi.base:
I don't think any of these need to be documented. If
containers
offer additional service methods, it's up to containers
to support
getting those methods into osapi. The SystemHandler
method you
describe above is reasonable.
no change. I don't fully understand this, but if containers
don't need to be consistent on the osapi.base namespace, then we can
leave this out
- Error Handling:
I like the pattern of using and propagating "error"
through the
structures. A big +1, as it's simple and clear. That
said, the doc
needs to be explicit that:
- "error" may not be used as a key to batch.add().
- "error" is reserved and may not be used as a
property of a JSON
RPC batch response item.
fixed
Also, when the doc says "The error handling works
exactly as it does
for the JSONRPC interface, since this api is based on
that protocol."
that's not true. In JSON-RPC, a network error means
that you get a
JSON structure with just an error:
{error: {message: "", code: ""}}
... without any per-request-item content. What you're
doing - which I
like - is creating dummy response items at each key and
copying the
network error into each response item.
no change - not sure how to update
In "Example: Processing any sub requests in the batch
that succeeded:
", the example is incorrect. We don't have an error
property within
each returned activity, since all activities are in a
single request.
We need an example like:
var batch = osapi.newBatch().
add('v', osapi.people.getViewer()).
add('vf', osapi.people.getViewerFriends());
batch.execute(function(result) {
for (var name in result) {
if (result.hasOwnProperty(name)) {
if (result[name].error) {
gadgets.log("Found an error: ",
result[name].error.message);
} else {
gadgets.log("Got some data: ", result[name]);
}
}
}
});
fixed
On Mon, Mar 30, 2009 at 12:15 PM, Bob Evans
<bobev...@google.com> wrote:
>
> The URL:
http://wiki.opensocial.org/index.php?title=OSAPI_Specification
>
> On Mon, Mar 30, 2009 at 12:01 PM, Bob Evans
<bobev...@google.com> wrote:
>> Hi,
>>
>> I have updated the OS Lite spec doc to match the
current
>> implementation (there is a follow on patch coming
today with the
>> metadata-based discovery of service methods).
>>
>> Known issues:
>>
>> * requestCreateActivity and activities.create. It
might be possible to
>> make all calls ui-interruptible, but this prevents
batching, well,
>> it makes batching a lot stranger and more involved.
For example, what
>> happens if I have a batch where 2 of 4 calls require
UI interaction
>> with the user. It's do-able, but would require a lot
more thought than
>> .9 allows, I think. My proposed solution keep
requestX methods, and
>> discuss making all service calls ui-intermediable in
the next version
>> spec discussion.
>>
>> * send vs execute vs call naming. I prefer execute,
but if people want
>> a shorter name, then send seems good. Some have
suggested call as a
>> name, but this is used in the language, so seems like
a bad idea.
>>
>> * Rename osapi.makeRequest to osapi.http.get/post. To
me, makeRequest
>> reflects the existing, underlying gadgets calls, but
I could see
>> making it http.
>>
>> * osapi.ui namespace is weird, better suggestions?
>>
>> * Should we have a bucket of params for the requestX
calls, or
>> positional params as it is now? Maybe all of this is
obviated if we
>> make the requestX calls be jsonrpc calls.
>>
>>
>> Thanks,
>> Bob
>>
>
> >
>
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google
Groups "OpenSocial and Gadgets Specification Discussion" group.
To post to this group, send email to
opensocial-and-gadgets-s...@googlegroups.com
To unsubscribe from this group, send email to
opensocial-and-gadgets-spec+unsubscr...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/opensocial-and-gadgets-spec?hl=en
-~----------~----~----~----~------~----~------~--~---
