Hi Ali Please read my inline comments.
On Fri, Dec 20, 2013 at 9:40 AM, Ali Lown <a...@lown.me.uk> wrote: > First a few random things from a quick glance over your code (I could put > them on ReviewBoard, or here is fine too): > - Please use IdConstants,X and DocumentConstants.X > - This will also encourage you to use IdUtil's methods - which will > 'correctly' distinguish conversation wavelets, blip documents etc. > Please also put your comment in the review. That will help me to locate the problem. Thanks. > - Your readText function, looks suspiciously similar to the collateX > functions in box.common.Snippets > I have a comment in the JavaDoc that the method was copied with modifications from Snippets. > - It only makes sense to index conversation wavelets (not user data > wavelets, robot wavelets etc.) > Can you review the latest patch? Let me know if the wavelets are filtered correctly for indexing. Thanks. > > For finding the title: > It only makes sense to check the root conversation wavelet for a title > (use IdUtil). > The 'correct' method would be to use ConversationUtil.buildConversation to > build the whole conversation structure for that wavelet, and to then follow > the root thread to find the first blip which will contain the title > annotation. This however could be very slow. > Since you already have the ReadableWaveletData for the root conversation > wavelet, it makes more sense for you to iterate over the blip documents > (IdUtil!), and then make use of > ReadableBlipData.getContent().getMutableDocument() which should return a > suitable document to pass to TitleHelper.extractTitle(). > Thanks for the information. I'll work on that. > > Ali > > P.S. Please try to keep wave-dev in the email chains. > > On 19 December 2013 23:45, Frank R. <renfeng...@gmail.com> wrote: > >> Yes. Just the title. >> >> org.waveprotocol.wave.model.conversation.TitleHelper.getTitle(WaveContext) >> requires an instance of >> org.waveprotocol.box.webclient.search.WaveContext >> >> org.waveprotocol.wave.model.conversation.TitleHelper.extractTitle(ReadableWDocument<N, >> E, T>) >> requires an instance of >> org.waveprotocol.wave.model.document.ReadableWDocument<N, E, T> >> >> The question is how to get those? All I have is an instance of >> org.waveprotocol.wave.model.wave.data.ReadableWaveletData >> and I can also get an instance of >> org.waveprotocol.wave.model.id.WaveletName >> >> Thanks~ >> >> >> On Fri, Dec 20, 2013 at 3:13 AM, Ali Lown <a...@lown.me.uk> wrote: >> >>> If you just want the title, take a look at >>> TitleHelper.getTitle(WaveContext) (wave/model/conversation). >>> >>> Ali >>> >>> >>> On 19 December 2013 14:18, Frank R. <renfeng...@gmail.com> wrote: >>> >>>> Does someone know how to get the annotation out of >>>> a ReadableWaveletData? I can use that to get the conv/title, and display it >>>> as link text in solr-bot search results. Thanks. >>>> >>>> >>>> On Thu, Dec 19, 2013 at 9:05 PM, Yuri Zelikov <vega...@gmail.com>wrote: >>>> >>>>> This is an automatically generated e-mail. To reply, visit: >>>>> https://reviews.apache.org/r/16322/ >>>>> >>>>> Thanks! >>>>> Can you please address the comments by closing them where done or >>>>> discarding where not relevant? >>>>> >>>>> >>>>> - Yuri Zelikov >>>>> >>>>> On December 19th, 2013, 12:23 p.m. UTC, Frank R. wrote: >>>>> Review request for wave, Ali Lown, Vicente J. Ruiz Jurado, and Yuri >>>>> Zelikov. >>>>> By Frank R.. >>>>> >>>>> *Updated Dec. 19, 2013, 12:23 p.m.* >>>>> *Repository: * wave >>>>> Description >>>>> >>>>> For details (issues and commits):https://github.com/renfeng/wave >>>>> >>>>> 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 >>>>> >>>>> 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) >>>>> >>>>> View Diff <https://reviews.apache.org/r/16322/diff/> >>>>> >>>> >>>> >>> >> >
