----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3564/#review4835 -----------------------------------------------------------
./build.xml <https://reviews.apache.org/r/3564/#comment10618> Please add line before the next target ./src/org/waveprotocol/box/server/ServerMain.java <https://reviews.apache.org/r/3564/#comment10619> 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. ./src/org/waveprotocol/box/server/waveserver/ImportServlet.java <https://reviews.apache.org/r/3564/#comment10624> Please change the name to GWAVE_PUBLIC_USER_DOMAIN "shared" has a bit different semantics. Also, please add spaces before and after "=" ./src/org/waveprotocol/box/server/waveserver/ImportServlet.java <https://reviews.apache.org/r/3564/#comment10620> Just add // HACK - Remove the delay after issue WAVE-328 will be fixed. ./src/org/waveprotocol/box/server/waveserver/ImportServlet.java <https://reviews.apache.org/r/3564/#comment10621> Please add some javadoc for the method. ./src/org/waveprotocol/box/server/waveserver/ImportServlet.java <https://reviews.apache.org/r/3564/#comment10622> DOes it mean that shared participant is not added to the wave? ./src/org/waveprotocol/box/server/waveserver/ImportServlet.java <https://reviews.apache.org/r/3564/#comment10623> Again, does it mean we don't handle the shared participant? ./src/org/waveprotocol/box/server/waveserver/ImportServlet.java <https://reviews.apache.org/r/3564/#comment10625> Please move up this constant definition. Also, please ad spaces before and after "=" ./src/org/waveprotocol/box/server/waveserver/ImportServlet.java <https://reviews.apache.org/r/3564/#comment10626> I think it would be better to call this method "isPublicParticipant" Also, I afraid there are some more users with this domain, like anonym...@a.gwave.com etc.. Which have nothing to do with wave ACL ./src/org/waveprotocol/box/server/waveserver/ImportServlet.java <https://reviews.apache.org/r/3564/#comment10627> 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. ./src/org/waveprotocol/box/server/waveserver/ImportServlet.java <https://reviews.apache.org/r/3564/#comment10628> That might not works correctly in case there are other participants from domain @a.gwave.com - and I think there are ./src/org/waveprotocol/box/waveimport/WaveExport.java <https://reviews.apache.org/r/3564/#comment10629> oauth_service -> oauthService ./src/org/waveprotocol/box/waveimport/WaveExport.java <https://reviews.apache.org/r/3564/#comment10630> Put the if() body inside a block {} ./src/org/waveprotocol/box/waveimport/WaveExport.java <https://reviews.apache.org/r/3564/#comment10631> waveledId -> waveletId ./src/org/waveprotocol/box/waveimport/WaveExport.java <https://reviews.apache.org/r/3564/#comment10632> fromVersion-1 -> fromVersion - 1 ./src/org/waveprotocol/box/waveimport/WaveImport.java <https://reviews.apache.org/r/3564/#comment10633> Consider using the split() method of String instead of StringTokenizer - Yuri On 2012-02-06 08:08:26, Andrew Kaplanov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3564/ > ----------------------------------------------------------- > > (Updated 2012-02-06 08:08:26) > > > 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 > ----- > > ./third_party/runtime/google_client/google-oauth-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-api-client-1.5.0-beta.jar > UNKNOWN > ./build.properties 1240026 > ./build.xml 1240026 > ./run-export.sh PRE-CREATION > ./run-import.sh PRE-CREATION > ./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 > ./README.import PRE-CREATION > > Diff: https://reviews.apache.org/r/3564/diff > > > Testing > ------- > > > Thanks, > > Andrew > >
