[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: RE: [sca-j] CAA Spec - WD-01 First Pass - RFC 2119 and Editorial Cleanup
|
Hi, Here are my review comments for the WD-01
from Mike dated 17th April 2008. I’ve not checked whether I have duplicated
any of Simon’s comments as I started the review before any of Simon’s
comments were posted on the list. Sorry about that. Editorial Footer Copyright should be 2007-2008? Page 2: Abstract: Bullet point 3 is on the end of Bullet
point 2 and has J3 at the start of it Page 7: Lines 8-9 Merge of bullet points 2 and 3 and 3 and 4.
They need to be separated out Page 7: Line 35-36 Version mismatch - the label says SDO 2.0
Specification but the link has SDO-Spec-v2.1.0 Page 7/8: Missing reference [5] which refers to the SCA
Policy document. See page 46 line 1621 and many other references after that point
in the document. Page 9: Lines 59 – 61 Page 12: Lines 177 - 179 No mention of operation overloading
disallowed. Should this be added as it is a key difference between local and remote
interfaces Page 9: Lines 59 – 61 Should an example be added? There is one
for the local case in the section immediately below it. Page 9: Line 73 Should be indented to be consistent with
the code in the rest of the document Page 15: Lines 253 – 254 Wrong import statements. Change line 253
to ConversationAttributes and delete line 254. Page 17: Lines 326 – 327 Bullet point is split over two lines. These
lines should be re-joined Page 18: Line 366 MAY NOT is not an RFC 2119 term Page 19: Lines 401 & 408 Page 30: Lines 914, 926 and 932 Page 35: Lines 1166 and 1167 Page 36: Lines 1227 & 1228 Page 43: Lines 1539, 1540 & 1541 Page 57: Line 2111 Remove redundant public keyword from
method on interface. Page 20: Lines 444 & 480 Page 25: Line 690 & 694 Page 27: Line 767 Page 31: Line 971 Page 37: Line 1263 Page 57 Line 2110 Missing space between end of class or method
definition and the “{“ character Page 20: Lines 477 – 495 Indentation inconsistent with the code
samples in the rest of the document (Basically move left lines 479 - 490 and
then move right lines 448 – 489) Page 21: Line 539 Page 22: Line 522 Wrong variable name - Should be
myCallback.receiveResult(theResult); Page 26: Lines 715 – 716 Inconsistent indentation of code.
(Basically move lit left) Page 27: Line 798 The text should say “The
Conversation interface” and not “The ServiceReference interface”
as we are talking about the Conversation interface. Page 28: Line 808 – 809 Inconsistent indentation. Move public
class … left and put ServiceRuntimeException on the line above as it
should now all fit on one line Page 28: Lines 803 – 811 Missing a description of this exception as
all the other exceptions have a short description at the end. Page 28: Line 812 – 821 and 803 –
811 Consider swapping Sections 7.6 No
Registered Callback Exception and 7.7 Service Runtime Exception over as No
Registered Callback Exception refers to Service Runtime Exception which has not
yet been described. Page 31: Line 957 & 986 Page 32: Lines 1009 & 1034 Page 33: Lines 1052 & 1078 Page 34: Lines 1093 & 1113 Page 35: Lines 1141 & 1164 Page 36: Line 1225 Page 40: Line 1384 Page 42: Lines 1456 & 1499 Page 43: Line 1537 Page 44: Line 1584 Two spaces between @interface and the
annotation. Remove the extra one Page 31: Line 967 Page 32: Line 1022 Page 44: Line 1594 Injection into private field –
thought this was not allowed. Should change field to protected. Page 31: Line 970 Add a new line above line 970 that says: “The following snippet shows a
component name setter method sample.” Page 32: Line 995 Add an example of how this annotation is
used. Many of the other annotations have examples Page 32: Line 1022 Add an example of injection into
RequestContext after line 1022. Perhaps the following text: “The following snippet shows a
RequestContext field definition sample. @Context protected RequestContext context;” Page 32: Line 1015 MUST BE is not an RFC 2119 keyword. Change
the BE to be lower case. Page 33: Line 1039 Should we add a sample of using the
@Conversational annotation? Page 33: Line 1080 Add a description of @EagerInit and an
example. Page 34: Line 1100 Should there be an example of using
@EndsConversation? Page 34: Line 1126 The method should be “public void
myInitMethod() {“ Page 35: Line 1149 Should there be an example of @OneWay? Page 36: Line 1195 setCurrency() is a method so should have {
and }. The text would become: public void
setCurrency( String theCurrency ) { … } Page 38: Line 1281 The Java sample does not define a package
declaration so the java.interface in the componentType could be argued to be
wrong. The way to fix this is to add a package statement in the sample code. E.g.,
add the following before line 1260 on page 37: package services.hello; Page 38: Lines 1290 – 1292 The start of this paragraph is phrased
differently to the same paragraph in Page 36 Lines 1197 – 1199. Should
these paragraphs use consistent language? I believe the language on Page 36 is
the correct version. Page 38: Line 1296, 1300 and 1320 In line 1296, we have helloServices (plural)
whereas the code and XML has helloService (singular). These should be made
consistent – probably plural? Page 38: Line 1305 – 1309 The code is missing a for loop. The code
refers to .get(index) but does not define it. This is important to add as
without it, it could be misinterpreted to imply that invoking a method on a
collection of references will invoke all the references. Add before line 1305: for (int index = 0; index < helloServices.size(); index++) { Add after line 1309: } Note: make sure that the name for
helloServices in the text added before line 1305 is consistent with the
decision on what happens with the helloService(s) on Page 38: Line 1296, 1300
and 1320 Page 39: Line 1361 Two .. (full stops) after “thrown an
exception as described above..” Remove one of them Page 41: Line 1437 Why does this sample have
@AllowsPassByReference? I think it should be removed as this sample is
demonstrating @Remotable Page 42: Line 1473: Page 43: Line 1516 Inconsistent indentation. Line needs to be
moved left Page 43: Line 1524 Should a sample showing @Service be added? Page 44: Line 1560 Add a paragraph break between 1560 and
1561 Page 44: Line 1561 Text is wrong. The text should be: “The following snippet shows the use
of @ConversationAttributes annotation to set the max age of a conversation to
be 30 days” Page 44: Line 1565 The rest of the document does not use
import …*; Change line 1565 to the following text to
make it consistent with the rest of the document. import org.osoa.sca.annotations.ConversationAttributes; Page 44: Line 1594 The spec says that the type of the field
is not necessarily a String (see Line 1596) so should we update the type of the
field in line 1594 to be of type Object rather than type String? Page 45: Line 1599 Reference [4] is to the WSDL
specifications and not the JAX-WS specification. I believe we need to add a new
reference for JAX-WS Page 45: Line 1602 The package name is wrong. It should be
org.osoa.annotations.OneWay (i.e. add annotations after osoa) Page 47: Lines 1681 and 1694 This is a method so should have { and } Page 47: Line 1699 Inconsistent indentation with the rest of
the document. Move the “where” left. Page 48: Lines 1725 and 1727 Page 54: Lines 1979, 1980, 1982, 2000 and
2002 Page 55: Lines 2004, 2022, 2024 and 2026 Remove redundant public static final
keywords off constants on the interface. Hopefully, lines 1725-1726, 1980-1981 and
1982-1983, 2000-2001, 2002-2003 2004-2005, 2022-2023, 2024-2025, 2026-2027will
now fit on one line each. Page 49: Line 1750 Inconsistent indentation with the rest of
the document. Move the “where” left. Page 51: Line 1851 Two import lines merged onto one line.
Split the import statements onto separate lines. Page 56: Line 2073 Why is example 1 after examples 2a and 2b
on Page 51 Line 1862 and Page 52 Line 1913 Also, why do we have these examples
labelled inconsistently with the rest of the document? None of the other
examples/samples are labelled by number. There is also Example 3 on Page 59 Line 2196 Page 60: Lines 2211 – 2212 I am presuming this will be filled in
later? Specification
Clarifications Page 24: Line 643 and Page 25: Line 668 Inconsistent description of return type on
method. Line 643
says: ServiceReference Line 668
says: CallableReference So what does it return? ServiceReference
or CallableReference? Page 26: Line 753 What does it return if reference is not
conversational? It does not specify anything. I am presuming that it will
return false. Add the text “and returns false if
this reference is not conversational” Page 36: Lines 1183 -1184 If name is not specified for @Property on
a setter method or Constructor parameter, what is the default value for name. The
text does not clarify these two scenarios. Page 37: Lines 1241 - 1242 If name is not specified for @Reference on
a setter method or Constructor parameter, what is the default value for name. The
text does not clarify these two scenarios. Page 38: Lines 1286 – 1288 Specification is not explicit with what
happens when required = false. I am assuming we simply need to add the
following text at the end of line 1288: Page 39: Lines 1342 & 1351 It talks about throwing InvalidServiceException
but this is not listed in Section 7 Java API. This exception should be defined
and added to that section Page 40: the ** in the table. There is some text in ** but there is no
text in the table above marked with **. Either this text should be removed or
linked into the correct bit of the text. Page 44: Line 1555 “The two attributes that take a time
express the time as a string that starts with an integer, …” This allows the time expression to be a negative
number or 0. Should the text be changed so that the number has to be > 0, so
change to: “The two attributes that take a
time express the time as a string that starts with a positive integer, …” Page 44: Lines 1555 – 1557 Am I allowed to use “proper English”?
– i.e. 1 second, 1 day, 1 hour and 1 year? The specification text
detailed here does not allow this. Page 44: Lines 1563 – 1570 There is an example of
@ConversationAttributes on a Service Implementation that is not marked as
@Scope(“CONVERSATION”). My questions are:
Thanks, Mark Mark Combellack| Software Developer| Avaya | From:
Mike Edwards [mailto:mike_edwards@uk.ibm.com]
Unless stated otherwise above:
|
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]