Hi Glen ,

Please see my comments inline.

Glen Mazza wrote:
Am Donnerstag, den 13.09.2007, 08:45 +0000 schrieb [EMAIL PROTECTED]:

Modified: 
incubator/cxf/trunk/tools/javato/ws/src/main/java/org/apache/cxf/tools/java2ws/JavaToWSContainer.java
URL: 
http://svn.apache.org/viewvc/incubator/cxf/trunk/tools/javato/ws/src/main/java/org/apache/cxf/tools/java2ws/JavaToWSContainer.java?rev=575222&r1=575221&r2=575222&view=diff
==============================================================================
--- 
incubator/cxf/trunk/tools/javato/ws/src/main/java/org/apache/cxf/tools/java2ws/JavaToWSContainer.java
 (original)
+++ 
incubator/cxf/trunk/tools/javato/ws/src/main/java/org/apache/cxf/tools/java2ws/JavaToWSContainer.java
 Thu Sep 13 01:45:21 2007
@@ -78,7 +78,7 @@

             if (!"simple".equalsIgnoreCase(ft) && 
!"jaxws".equalsIgnoreCase(ft)) {
- Message msg = new Message("INVALID_FORNTEND", LOG, new Object[]{ft}); + Message msg = new Message("INVALID_FRONTEND", LOG, new Object[] {ft});
                 errs.add(new ErrorVisitor.UserError(msg.toString()));
             }
         }

I think we should make the constants--"jaxws" and
"simple"--case-sensitive, it will help us in the future should we use
constants for these two values.  Also, our command-line scripts are
already case-sensitive (correct?), and Unix-like case sensitivity gives
a good feeling to the user that the scripts are very rigorous and
robust.

+1.

+                Message msg = new Message("WRAPPERBEAN_WITHOUT_JAXWS", LOG);
                 errs.add(new ErrorVisitor.UserError(msg.toString()));
             }
         }
-
Modified: 
incubator/cxf/trunk/tools/javato/ws/src/main/java/org/apache/cxf/tools/java2ws/Messages.properties
URL: 
http://svn.apache.org/viewvc/incubator/cxf/trunk/tools/javato/ws/src/main/java/org/apache/cxf/tools/java2ws/Messages.properties?rev=575222&r1=575221&r2=575222&view=diff
==============================================================================
--- 
incubator/cxf/trunk/tools/javato/ws/src/main/java/org/apache/cxf/tools/java2ws/Messages.properties
 (original)
+++ 
incubator/cxf/trunk/tools/javato/ws/src/main/java/org/apache/cxf/tools/java2ws/Messages.properties
 Thu Sep 13 01:45:21 2007
@@ -23,6 +23,7 @@
 FILE_NOT_EXIST = File does not exist
 NOT_A_FILE = {0} is not a file
 PARAMETER_MISSING = Required parameter is missing or not valid
-INVALID_FORNTEND = \ "{0}" is not a valid frontend, java2ws only supports 
jaxws and simple frontend.
-CANT_GEN_WRAPPERBEAN = Wrapperbean only needs to be generated for jaxws front 
end.
+INVALID_FRONTEND = "{0}" is not a valid frontend, java2ws only supports jaxws 
and the simple frontend.

INVALID_FRONTEND = "{0}" is not a valid frontend, java2ws only supports
the "simple" and "jaxws" frontend.

We need quotes so the reader knows what to type in; also, if we put
simple last it seems to be a sarcastic comment (i.e., "simple" frontend
can mean not-very-simple front end)

+1. I think it is a good practice to quote the reader need to type in .
+WRAPPERBEAN_WITHOUT_JAXWS = -wrapperbean is only valid for the jaxws front end.
+INVALID_DATABINDING = Invalid value {0} for data binding type.
Modified: 
incubator/cxf/trunk/tools/javato/ws/src/main/java/org/apache/cxf/tools/java2ws/java2ws.xml
URL: 
http://svn.apache.org/viewvc/incubator/cxf/trunk/tools/javato/ws/src/main/java/org/apache/cxf/tools/java2ws/java2ws.xml?rev=575222&r1=575221&r2=575222&view=diff
==============================================================================
--- 
incubator/cxf/trunk/tools/javato/ws/src/main/java/org/apache/cxf/tools/java2ws/java2ws.xml
 (original)
+++ 
incubator/cxf/trunk/tools/javato/ws/src/main/java/org/apache/cxf/tools/java2ws/java2ws.xml
 Thu Sep 13 01:45:21 2007
@@ -35,9 +35,19 @@
        </annotation>
        <usage>
                <optionGroup id="options">
+               
+                       <option id="databinding" maxOccurs="1">
+                               <annotation>
+                                   Specify the data binding (aegis or jaxb). 
Default jaxb.
+                               </annotation>
+                               <switch>databinding</switch>
+                               <associatedArgument placement="afterSpace">
+                                 <annotation>jaxb or aegis</annotation>
+                               </associatedArgument>
+                       </option>
                        <option id="frontend" maxOccurs="1">
                                <annotation>
-                                       specify which frontend should be use, 
support jaxws and simple frontend.
+                                       Specify which frontend should be use, 
support jaxws and simple frontend.

Specify the frontend to use, either "simple" or "jaxws" frontend.


                                </annotation>
                                <switch>frontend</switch>
                                <associatedArgument placement="afterSpace">


Modified:
incubator/cxf/trunk/tools/javato/ws/src/main/java/org/apache/cxf/tools/java2wsdl/processor/internal/ServiceBuilderFactory.java
URL:
http://svn.apache.org/viewvc/incubator/cxf/trunk/tools/javato/ws/src/main/java/org/apache/cxf/tools/java2wsdl/processor/internal/ServiceBuilderFactory.java?rev=575222&r1=575221&r2=575222&view=diff
==============================================================================
---
incubator/cxf/trunk/tools/javato/ws/src/main/java/org/apache/cxf/tools/java2wsdl/processor/internal/ServiceBuilderFactory.java
 (original)
+++
incubator/cxf/trunk/tools/javato/ws/src/main/java/org/apache/cxf/tools/java2wsdl/processor/internal/ServiceBuilderFactory.java
 Thu Sep 13 01:45:21 2007
@@ -19,17 +19,28 @@
- public ServiceBuilder newBuilder(FrontendFactory.Style s) {
+        String beanName = getBuilderBeanName(s);
         ServiceBuilder builder = null;
+
         try {
-            String clzName = getBuilderClassName(s);
-            builder = (ServiceBuilder)
Class.forName(clzName).newInstance();
-        } catch (Exception e) {
-            throw new ToolException("Can not find or initialize the
ServiceBuilder for style: " + s
+            builder = (ServiceBuilder)
applicationContext.getBean(beanName, ServiceBuilder.class);
+            AbstractServiceFactory serviceFactory =
(AbstractServiceFactory)builder;
+            serviceFactory.setDataBinding(dataBinding);
+        } catch (RuntimeException e) {
+            throw new ToolException("Can not get ServiceBuilder bean
" + beanName + + "to initialize the
ServiceBuilder for style: " + s

" to initialize..." (leading space needed)


Added: 
incubator/cxf/trunk/tools/javato/ws/src/test/java/org/apache/cxf/tools/fortest/aegis2ws/Something.java
URL: 
http://svn.apache.org/viewvc/incubator/cxf/trunk/tools/javato/ws/src/test/java/org/apache/cxf/tools/fortest/aegis2ws/Something.java?rev=575222&view=auto
==============================================================================
--- 
incubator/cxf/trunk/tools/javato/ws/src/test/java/org/apache/cxf/tools/fortest/aegis2ws/Something.java
 (added)
+++ 
incubator/cxf/trunk/tools/javato/ws/src/test/java/org/apache/cxf/tools/fortest/aegis2ws/Something.java
 Thu Sep 13 01:45:21 2007
@@ -0,0 +1,52 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.cxf.tools.fortest.aegis2ws;
+
+/**
+ * Test data type for Aegis in java2ws
+ */
+public class Something {
+    private String multiple;
+    private String singular;
+

Is there any semantic meaning for multiple and singular?  The reader
gets that impression, but does know what those words mean.  If there
*is* a meaning, a quick comment in this file of what a single and a
multiple would be good.  If there are *no* meanings, usually "foo" and
"bar" are better to indicate that.


This class is for test case and come from Benson's patch . I think it is good to change it to "bar" and "foo".
Benson , do you think so ?


Thanks

Jim


Reply via email to