> On 2012-01-29 11:10:13, Yuri Zelikov wrote: > > ./src/org/waveprotocol/box/server/waveserver/ImportServlet.java, line 167 > > <https://reviews.apache.org/r/3564/diff/5/?file=71213#file71213line167> > > > > Can we refactor the code in this "else" into a method? > > handleAddParticipantOp() (also for remove participant)
done > On 2012-01-29 11:10:13, Yuri Zelikov wrote: > > ./src/org/waveprotocol/box/server/waveserver/ImportServlet.java, line 197 > > <https://reviews.apache.org/r/3564/diff/5/?file=71213#file71213line197> > > > > Can we use here some util to construct the path? Probably > > ModernIdSerializer has what we need here. Used HashedVersionFactory instead. > On 2012-01-29 11:10:13, Yuri Zelikov wrote: > > ./src/org/waveprotocol/box/server/waveserver/ImportServlet.java, line 210 > > <https://reviews.apache.org/r/3564/diff/5/?file=71213#file71213line210> > > > > I think we need to add some tests for the code. The convertDelta() > > method is very important and probably it would be nice to add unit tests to > > make sure it works as expected. done > On 2012-01-29 11:10:13, Yuri Zelikov wrote: > > ./src/org/waveprotocol/box/server/waveserver/ImportServlet.java, line 216 > > <https://reviews.apache.org/r/3564/diff/5/?file=71213#file71213line216> > > > > Same here. Can we add unit test for this method? done > On 2012-01-29 11:10:13, Yuri Zelikov wrote: > > ./src/org/waveprotocol/box/waveimport/WaveExport.java, line 163 > > <https://reviews.apache.org/r/3564/diff/5/?file=71214#file71214line163> > > > > How about the case when the user passed explicit list of wave ids he > > wants to import? I think in such case we can just fetch only the requested > > waves, without scanning all the inbox. Reading of all waves list was excluded for such cause. > On 2012-01-29 11:10:13, Yuri Zelikov wrote: > > ./src/org/waveprotocol/box/waveimport/WaveImport.java, line 106 > > <https://reviews.apache.org/r/3564/diff/5/?file=71215#file71215line106> > > > > How about changing signature of this method to void, and throwing > > exception in case there is a problem? It seems like the exception mechanism > > will work better here compared to returning status in the output. > > Also, according to Java conventions, methods that return boolean should > > start with "is, has" etc... Changed signature of this method to String with reply from service. - Andrew ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3564/#review4676 ----------------------------------------------------------- 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 > >
