> On 2012-02-06 17:13:20, Yuri Zelikov wrote: > > ./src/org/waveprotocol/box/server/waveserver/ImportServlet.java, line 204 > > <https://reviews.apache.org/r/3564/diff/7/?file=72577#file72577line204> > > > > DOes it mean that shared participant is not added to the wave?
yes > On 2012-02-06 17:13:20, Yuri Zelikov wrote: > > ./src/org/waveprotocol/box/server/waveserver/ImportServlet.java, line 242 > > <https://reviews.apache.org/r/3564/diff/7/?file=72577#file72577line242> > > > > That might not works correctly in case there are other participants > > from domain @a.gwave.com - and I think there are Do you mind people in domain @a.gwave.com, do you think they are? I think that domain reserved by Google for they public participants :) > On 2012-02-06 17:13:20, Yuri Zelikov wrote: > > ./src/org/waveprotocol/box/waveimport/WaveImport.java, line 90 > > <https://reviews.apache.org/r/3564/diff/7/?file=72579#file72579line90> > > > > Consider using the split() method of String instead of StringTokenizer Why? split() returns array of String and I need to use index variable to get items from array. Use of the index less clear and fraught with more errors. > On 2012-02-06 17:13:20, Yuri Zelikov wrote: > > ./src/org/waveprotocol/box/server/waveserver/ImportServlet.java, line 221 > > <https://reviews.apache.org/r/3564/diff/7/?file=72577#file72577line221> > > > > Again, does it mean we don't handle the shared participant? Yes. I asked you about shared participants. First you say that Wiab does not have shared participants, next you say thet Wiab "has the so called "shared participant" (@yourwavedomain.com), which functions similar to the [email protected]." I tried to use "shared participant" and have received an error: java.io.IOException: shared participant@localhost is not a participant of [WaveletName localhost/w+b8y1ckplA/localhost/conv+root] > On 2012-02-06 17:13:20, Yuri Zelikov wrote: > > ./src/org/waveprotocol/box/server/waveserver/ImportServlet.java, line 64 > > <https://reviews.apache.org/r/3564/diff/7/?file=72577#file72577line64> > > > > Please change the name to GWAVE_PUBLIC_USER_DOMAIN > > "shared" has a bit different semantics. > > > > Also, please add spaces before and after "=" Let this true, then why the same user in Wiab you called "shared participant"? > On 2012-02-06 17:13:20, Yuri Zelikov wrote: > > ./src/org/waveprotocol/box/server/waveserver/ImportServlet.java, line 240 > > <https://reviews.apache.org/r/3564/diff/7/?file=72577#file72577line240> > > > > String perticipant -> String participant > > > > Also, please move this method up. Public static methods should be > > defined after member fields definition but before the constructor. I > > personally don't care if you leave private static methods here. changed to "private" > On 2012-02-06 17:13:20, Yuri Zelikov wrote: > > ./src/org/waveprotocol/box/waveimport/WaveExport.java, line 159 > > <https://reviews.apache.org/r/3564/diff/7/?file=72578#file72578line159> > > > > oauth_service -> oauthService done > On 2012-02-06 17:13:20, Yuri Zelikov wrote: > > ./src/org/waveprotocol/box/waveimport/WaveExport.java, line 162 > > <https://reviews.apache.org/r/3564/diff/7/?file=72578#file72578line162> > > > > Put the if() body inside a block {} done > On 2012-02-06 17:13:20, Yuri Zelikov wrote: > > ./src/org/waveprotocol/box/waveimport/WaveExport.java, line 172 > > <https://reviews.apache.org/r/3564/diff/7/?file=72578#file72578line172> > > > > waveledId -> waveletId done > On 2012-02-06 17:13:20, Yuri Zelikov wrote: > > ./src/org/waveprotocol/box/waveimport/WaveExport.java, line 192 > > <https://reviews.apache.org/r/3564/diff/7/?file=72578#file72578line192> > > > > fromVersion-1 -> fromVersion - 1 done > On 2012-02-06 17:13:20, Yuri Zelikov wrote: > > ./src/org/waveprotocol/box/server/waveserver/ImportServlet.java, line 234 > > <https://reviews.apache.org/r/3564/diff/7/?file=72577#file72577line234> > > > > Please move up this constant definition. > > Also, please ad spaces before and after "=" done > On 2012-02-06 17:13:20, Yuri Zelikov wrote: > > ./src/org/waveprotocol/box/server/waveserver/ImportServlet.java, line 163 > > <https://reviews.apache.org/r/3564/diff/7/?file=72577#file72577line163> > > > > Please add some javadoc for the method. done > On 2012-02-06 17:13:20, Yuri Zelikov wrote: > > ./src/org/waveprotocol/box/server/ServerMain.java, line 233 > > <https://reviews.apache.org/r/3564/diff/7/?file=72575#file72575line233> > > > > Would be better if we could enable/disable the import portlet based on > > a flag. But it doesn't has to be implemented in this patch. done > On 2012-02-06 17:13:20, Yuri Zelikov wrote: > > ./build.xml, line 662 > > <https://reviews.apache.org/r/3564/diff/7/?file=72572#file72572line662> > > > > Please add line before the next target But targets dist-server and dist-server-dep are not separated by empty line. I tried to match the style of. - Andrew ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3564/#review4835 ----------------------------------------------------------- On 2012-02-07 12:43:00, Andrew Kaplanov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3564/ > ----------------------------------------------------------- > > (Updated 2012-02-07 12:43:00) > > > Review request for wave and Yuri Zelikov. > > > Summary > ------- > > Exports waves from GWave to files, and imports them to Wiab by two command > line utilities. > The implementation is very raw, due to time constraints. > Import accesses special service on the Wiab. > Authorization on import service are not implemented now. > Imports of attachments not yet implemented. > Also, there is no generation of wave Id's for import into a non-empty store, > and from different domains. > Instructions for use enclosed in the file README.import. > > > Diffs > ----- > > ./README.import PRE-CREATION > ./build.properties 1240026 > ./build.xml 1240026 > ./run-export.sh PRE-CREATION > ./run-import.sh PRE-CREATION > ./server-config.xml 1240026 > ./src/org/waveprotocol/box/server/CoreSettings.java 1240026 > ./src/org/waveprotocol/box/server/ServerMain.java 1240026 > ./src/org/waveprotocol/box/server/util/testing/TestingConstants.java > 1240026 > ./src/org/waveprotocol/box/server/waveserver/ImportServlet.java > PRE-CREATION > ./src/org/waveprotocol/box/waveimport/WaveExport.java PRE-CREATION > ./src/org/waveprotocol/box/waveimport/WaveImport.java PRE-CREATION > ./src/org/waveprotocol/box/waveimport/google/RobotApi.java PRE-CREATION > ./src/org/waveprotocol/box/waveimport/google/RobotSearchDigest.java > PRE-CREATION > ./src/org/waveprotocol/box/waveimport/google/RobotSearchDigestGsonImpl.java > PRE-CREATION > ./src/org/waveprotocol/box/waveimport/google/RobotSearchDigestImpl.java > PRE-CREATION > ./src/org/waveprotocol/box/waveimport/google/RobotSearchDigestUtil.java > PRE-CREATION > > ./src/org/waveprotocol/box/waveimport/google/oauth/NeedNewOAuthTokenException.java > PRE-CREATION > ./src/org/waveprotocol/box/waveimport/google/oauth/OAuthCredentials.java > PRE-CREATION > ./src/org/waveprotocol/box/waveimport/google/oauth/OAuthRequestHelper.java > PRE-CREATION > ./src/org/waveprotocol/box/waveimport/google/oauth/OAuthedFetchService.java > PRE-CREATION > ./src/org/waveprotocol/box/waveimport/google/oauth/StableUserId.java > PRE-CREATION > ./src/org/waveprotocol/box/waveimport/google/oauth/UserContext.java > PRE-CREATION > ./test/org/waveprotocol/box/server/waveserver/ImportServletTest.java > PRE-CREATION > ./third_party/runtime/google_client/COPYING PRE-CREATION > ./third_party/runtime/google_client/google-api-client-1.5.0-beta.jar > UNKNOWN > ./third_party/runtime/google_client/google-http-client-1.5.0-beta.jar > UNKNOWN > ./third_party/runtime/google_client/google-oauth-client-1.5.0-beta.jar > UNKNOWN > > Diff: https://reviews.apache.org/r/3564/diff > > > Testing > ------- > > > Thanks, > > Andrew > >
