Multiple and singular are meaningful. See the .aegis.xml file that sets maxOccurs for them: singular has maxOccurs=1, multiple is unbounded. So I think these names make the test case that uses them more readable.
> -----Original Message----- > From: Jim Ma [mailto:[EMAIL PROTECTED] > Sent: Sunday, September 16, 2007 10:25 PM > To: [email protected] > Subject: Re: svn commit: r575222 - in /incubator/cxf/trunk: > api/src/main/java/org/apache/cxf/tools/common/ tools/common/ > tools/common/src/main/java/org/apache/cxf/tools/common/ tools/javato/ > tools/javato/ws/ tools/javato/ws/src/main/java/org/apache/cxf/tools/java2 > > 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/j av > a2ws/JavaToWSContainer.java > >> URL: > http://svn.apache.org/viewvc/incubator/cxf/trunk/tools/javato/ws/src/mai n/ > java/org/apache/cxf/tools/java2ws/JavaToWSContainer.java?rev=575222&r1=5 75 > 221&r2=575222&view=diff > >> > ======================================================================== == > ==== > >> --- > incubator/cxf/trunk/tools/javato/ws/src/main/java/org/apache/cxf/tools/j av > a2ws/JavaToWSContainer.java (original) > >> +++ > incubator/cxf/trunk/tools/javato/ws/src/main/java/org/apache/cxf/tools/j av > a2ws/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/j av > a2ws/Messages.properties > >> URL: > http://svn.apache.org/viewvc/incubator/cxf/trunk/tools/javato/ws/src/mai n/ > java/org/apache/cxf/tools/java2ws/Messages.properties?rev=575222&r1=5752 21 > &r2=575222&view=diff > >> > ======================================================================== == > ==== > >> --- > incubator/cxf/trunk/tools/javato/ws/src/main/java/org/apache/cxf/tools/j av > a2ws/Messages.properties (original) > >> +++ > incubator/cxf/trunk/tools/javato/ws/src/main/java/org/apache/cxf/tools/j av > a2ws/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/j av > a2ws/java2ws.xml > >> URL: > http://svn.apache.org/viewvc/incubator/cxf/trunk/tools/javato/ws/src/mai n/ > java/org/apache/cxf/tools/java2ws/java2ws.xml?rev=575222&r1=575221&r2=57 52 > 22&view=diff > >> > ======================================================================== == > ==== > >> --- > incubator/cxf/trunk/tools/javato/ws/src/main/java/org/apache/cxf/tools/j av > a2ws/java2ws.xml (original) > >> +++ > incubator/cxf/trunk/tools/javato/ws/src/main/java/org/apache/cxf/tools/j av > a2ws/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/j av > a2wsdl/processor/internal/ServiceBuilderFactory.java > >> URL: > >> > http://svn.apache.org/viewvc/incubator/cxf/trunk/tools/javato/ws/src/mai n/ > java/org/apache/cxf/tools/java2wsdl/processor/internal/ServiceBuilderFac to > ry.java?rev=575222&r1=575221&r2=575222&view=diff > >> > ======================================================================== == > ==== > >> --- > >> > incubator/cxf/trunk/tools/javato/ws/src/main/java/org/apache/cxf/tools/j av > a2wsdl/processor/internal/ServiceBuilderFactory.java (original) > >> +++ > >> > incubator/cxf/trunk/tools/javato/ws/src/main/java/org/apache/cxf/tools/j av > a2wsdl/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/f or > test/aegis2ws/Something.java > >> URL: > http://svn.apache.org/viewvc/incubator/cxf/trunk/tools/javato/ws/src/tes t/ > java/org/apache/cxf/tools/fortest/aegis2ws/Something.java?rev=575222&vie w= > auto > >> > ======================================================================== == > ==== > >> --- > incubator/cxf/trunk/tools/javato/ws/src/test/java/org/apache/cxf/tools/f or > test/aegis2ws/Something.java (added) > >> +++ > incubator/cxf/trunk/tools/javato/ws/src/test/java/org/apache/cxf/tools/f or > test/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 >
