> On Dec. 23, 2013, 10:28 a.m., Ali Lown wrote: > > Thanks Frank. > > > > We are getting a lot closer now, here are my semantic comments for the > > moment. (Yuri seems to be taking care of the syntactic stuff more).
Thanks. This is encouraging~ > On Dec. 23, 2013, 10:28 a.m., Ali Lown wrote: > > src/org/waveprotocol/box/server/waveserver/SolrWaveIndexerImpl.java, line > > 220 > > <https://reviews.apache.org/r/16322/diff/8/?file=402000#file402000line220> > > > > Please ensure the wavelet is a root conversation wavelet here, using > > the appropriately named method in IdUtils. It won't be necessary here. This is only a comment. I'll do it for SolrRobot of the same problem. > On Dec. 23, 2013, 10:28 a.m., Ali Lown wrote: > > src/org/waveprotocol/box/server/waveserver/SolrWaveIndexerImpl.java, line > > 275 > > <https://reviews.apache.org/r/16322/diff/8/?file=402000#file402000line275> > > > > We have the actual wavelet name, don't put this string-thing here. Is this equivalent to waveName? waveId + "/~/conv+root/" I read waveName as the following with breakpoint-and-watch. local.net/w+sfuoRAt-oCA/local.net/conv+root Anyway, this is for recreating the links as you can find by clicking the Link button on the upper right corner of a wave editor. I always see things like wave://local.net/w+sfuoRAt-oCA/~/conv+root/b+sfuoRAt-oCB http://localhost:9898/waveref/local.net/w+sfuoRAt-oCA/~/conv+root/b+sfuoRAt-oCB Perhaps, I should look into the client side (gwt) code, and see how the link is stringed together. > On Dec. 23, 2013, 10:28 a.m., Ali Lown wrote: > > src/org/waveprotocol/box/server/waveserver/SolrWaveIndexerImpl.java, lines > > 232-237 > > <https://reviews.apache.org/r/16322/diff/8/?file=402000#file402000line232> > > > > Use the appropriately named method in IdUtils to determine if this is a > > blip document, rather than this (incorrect) hack. I found "conversation".equals(docName) an equivalent to org.waveprotocol.wave.model.id.IdUtil.isManifestDocument(String) For "m/read".equals(docName), I got only this org.waveprotocol.wave.model.supplement.WaveletBasedSupplement.READSTATE_DOCUMENT Perhaps, I should have covered all the document names listed in org.waveprotocol.wave.model.supplement.WaveletBasedSupplement Anyway, am I supposed to replace the whole condition with this? org.waveprotocol.wave.model.id.IdUtil.isConversationRootWaveletId(WaveletId) > On Dec. 23, 2013, 10:28 a.m., Ali Lown wrote: > > src/org/waveprotocol/box/server/waveserver/SolrWaveIndexerImpl.java, lines > > 310-377 > > <https://reviews.apache.org/r/16322/diff/8/?file=402000#file402000line310> > > > > Why is this code still here? > > Remove from the patch. Sorry, I didn't read your last comment. Will do. Thanks~ - Frank ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16322/#review30803 ----------------------------------------------------------- On Dec. 23, 2013, 1:10 a.m., Frank R. wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16322/ > ----------------------------------------------------------- > > (Updated Dec. 23, 2013, 1:10 a.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/com/google/wave/api/Annotation.java b55f778 > 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. > >
