----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16322/#review30763 -----------------------------------------------------------
Some of these overlap with Yuri's, but most look to be new to me. Please actually mark the issues as fixed/resolved for each one. Thanks. src/org/waveprotocol/box/server/robots/agent/search/SolrRobot.java <https://reviews.apache.org/r/16322/#comment58879> This comment can be remove because of below. src/org/waveprotocol/box/server/robots/agent/search/SolrRobot.java <https://reviews.apache.org/r/16322/#comment58878> Same as below. src/org/waveprotocol/box/server/robots/agent/search/SolrRobot.java <https://reviews.apache.org/r/16322/#comment58877> These are defined in wave.client.doodad.selection.SelectionAnnotationHandler. Can you put these in a file in wave.model.conversation and then refer to them from there? src/org/waveprotocol/box/server/robots/agent/search/SolrRobot.java <https://reviews.apache.org/r/16322/#comment58880> Can you remove these comments and the disabled code. src/org/waveprotocol/box/server/robots/agent/search/SolrRobot.java <https://reviews.apache.org/r/16322/#comment58881> remove. src/org/waveprotocol/box/server/robots/agent/search/SolrRobot.java <https://reviews.apache.org/r/16322/#comment58882> a) is this always actually a new line? b) remove comment once fixed src/org/waveprotocol/box/server/robots/agent/search/SolrRobot.java <https://reviews.apache.org/r/16322/#comment58883> remove comment src/org/waveprotocol/box/server/robots/agent/search/SolrRobot.java <https://reviews.apache.org/r/16322/#comment58884> remove comment + dead code src/org/waveprotocol/box/server/robots/agent/search/SolrRobot.java <https://reviews.apache.org/r/16322/#comment58885> At least move these to constants at the top. (Though style stuff shouldn't be here) src/org/waveprotocol/box/server/robots/agent/search/SolrRobot.java <https://reviews.apache.org/r/16322/#comment58886> remove src/org/waveprotocol/box/server/robots/agent/search/SolrRobot.java <https://reviews.apache.org/r/16322/#comment58887> remove comment and dead code src/org/waveprotocol/box/server/robots/agent/search/SolrRobot.java <https://reviews.apache.org/r/16322/#comment58888> hard-coded colours again. src/org/waveprotocol/box/server/waveserver/SimpleSearchProviderImpl.java <https://reviews.apache.org/r/16322/#comment58889> trailing space src/org/waveprotocol/box/server/waveserver/SolrSearchProviderImpl.java <https://reviews.apache.org/r/16322/#comment58890> trailing space src/org/waveprotocol/box/server/waveserver/SolrSearchProviderImpl.java <https://reviews.apache.org/r/16322/#comment58891> Definitely. Update CoreSettings, then Inject the setting in to this class. src/org/waveprotocol/box/server/waveserver/SolrSearchProviderImpl.java <https://reviews.apache.org/r/16322/#comment58892> remove comment and commented line src/org/waveprotocol/box/server/waveserver/SolrSearchProviderImpl.java <https://reviews.apache.org/r/16322/#comment58893> err? src/org/waveprotocol/box/server/waveserver/SolrSearchProviderImpl.java <https://reviews.apache.org/r/16322/#comment58894> remove src/org/waveprotocol/box/server/waveserver/SolrSearchProviderImpl.java <https://reviews.apache.org/r/16322/#comment58895> remove src/org/waveprotocol/box/server/waveserver/SolrSearchProviderImpl.java <https://reviews.apache.org/r/16322/#comment58896> remove src/org/waveprotocol/box/server/waveserver/SolrSearchProviderImpl.java <https://reviews.apache.org/r/16322/#comment58897> remove src/org/waveprotocol/box/server/waveserver/SolrWaveIndexerImpl.java <https://reviews.apache.org/r/16322/#comment58898> trailing whitespace src/org/waveprotocol/box/server/waveserver/SolrWaveIndexerImpl.java <https://reviews.apache.org/r/16322/#comment58899> This looks an awful lot like the collateX in box.common.Snippets. DRY? src/org/waveprotocol/box/server/waveserver/SolrWaveIndexerImpl.java <https://reviews.apache.org/r/16322/#comment58902> It only makes sense to updateIndex for the root conversation wavelet. Add a precondition/check here using IdUtil.isConversationRootWaveletId() src/org/waveprotocol/box/server/waveserver/SolrWaveIndexerImpl.java <https://reviews.apache.org/r/16322/#comment58900> remove src/org/waveprotocol/box/server/waveserver/SolrWaveIndexerImpl.java <https://reviews.apache.org/r/16322/#comment58903> It is much easier to use IdUtil.isBlipId to check instead... src/org/waveprotocol/box/server/waveserver/SolrWaveIndexerImpl.java <https://reviews.apache.org/r/16322/#comment58901> remove src/org/waveprotocol/box/server/waveserver/SolrWaveIndexerImpl.java <https://reviews.apache.org/r/16322/#comment58904> remove src/org/waveprotocol/box/server/waveserver/SolrWaveIndexerImpl.java <https://reviews.apache.org/r/16322/#comment58905> remove src/org/waveprotocol/box/server/waveserver/SolrWaveIndexerImpl.java <https://reviews.apache.org/r/16322/#comment58906> remove - Ali Lown On Dec. 20, 2013, 5:12 p.m., Frank R. wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16322/ > ----------------------------------------------------------- > > (Updated Dec. 20, 2013, 5:12 p.m.) > > > Review request for wave, Ali Lown, Vicente J. Ruiz Jurado, and Yuri Zelikov. > > > Bugs: WAVE-311 > https://issues.apache.org/jira/browse/WAVE-311 > > > Repository: wave > > > Description > ------- > > For details (issues and commits): > https://github.com/renfeng/wave > > > Diffs > ----- > > run-export.sh d2cddb7 > run-import.sh 45fff8a > server.config.example 19ba8b2 > src/org/waveprotocol/box/server/SearchModule.java 2de0ef9 > src/org/waveprotocol/box/server/ServerMain.java b50454d > src/org/waveprotocol/box/server/robots/agent/search/SolrRobot.java > PRE-CREATION > src/org/waveprotocol/box/server/robots/agent/welcome/WelcomeRobot.java > 2735940 > src/org/waveprotocol/box/server/waveserver/SimpleSearchProviderImpl.java > ee7093f > src/org/waveprotocol/box/server/waveserver/SolrSearchProviderImpl.java > PRE-CREATION > src/org/waveprotocol/box/server/waveserver/SolrWaveIndexerImpl.java > PRE-CREATION > src/org/waveprotocol/box/server/waveserver/WaveDigester.java b103bbb > > Diff: https://reviews.apache.org/r/16322/diff/ > > > Testing > ------- > > tests on search box > > * in:inbox > * (empty) for all, including waves shared in the domain > * with:@ > * (free texts) > > tests on solr-bot > > * single word > * phrase (quoted with double quotation marks) > * syntax applicable to search box > > > Thanks, > > Frank R. > >
