incubator-wave-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Frank R." <renfeng...@gmail.com>
Subject Re: Review Request 16322: full text search (with known issue of solr-bot output highlight)
Date Sat, 21 Dec 2013 16:23:42 GMT


> On Dec. 19, 2013, 3:47 a.m., Yuri Zelikov wrote:
> > src/org/waveprotocol/box/server/SearchModule.java, line 79
> > <https://reviews.apache.org/r/16322/diff/2/?file=398899#file398899line79>
> >
> >     Seems like this comment is can be removed.

I'd like to keep it there as a reminder that PerUserWaveViewBus doesn't apply to every wave
indexer. Just because it was assumed so I had to bind it to a class with dummy methods.


> On Dec. 19, 2013, 3:47 a.m., Yuri Zelikov wrote:
> > src/org/waveprotocol/box/server/robots/agent/search/SolrRobot.java, line 3
> > <https://reviews.apache.org/r/16322/diff/2/?file=398901#file398901line3>
> >
> >     Please remove blank lines

The dual blank lines were supposed to be reduced to one upon formatting. I see this was corrected
by https://reviews.apache.org/r/16434

<setting id="org.eclipse.jdt.core.formatter.number_of_empty_lines_to_preserve" value="1"/>


> On Dec. 19, 2013, 3:47 a.m., Yuri Zelikov wrote:
> > src/org/waveprotocol/box/server/robots/agent/search/SolrRobot.java, line 163
> > <https://reviews.apache.org/r/16322/diff/2/?file=398901#file398901line163>
> >
> >     Do we really need the comment here?

It was experimental whether to take only the last line users typed as the search query. Better
to keep it until the decision will be permanent.


> On Dec. 19, 2013, 3:47 a.m., Yuri Zelikov wrote:
> > src/org/waveprotocol/box/server/robots/agent/search/SolrRobot.java, line 172
> > <https://reviews.apache.org/r/16322/diff/2/?file=398901#file398901line172>
> >
> >     Do we really need the comment here?

To prevent me and someone else from enabling that code again, and replace what works but is
ugly, maybe.


> On Dec. 19, 2013, 3:47 a.m., Yuri Zelikov wrote:
> > src/org/waveprotocol/box/server/robots/agent/search/SolrRobot.java, line 187
> > <https://reviews.apache.org/r/16322/diff/2/?file=398901#file398901line187>
> >
> >     Do we really need the comment here?

It's experimental to restrict which waves to include in search results. The search query is
entered in a wave shared with others, whose wave should be displayed, the creator's or the
other participant's? It should be similar problem with passwd-bot. Currently, only the creator's
waves are displayed. So, the creator's presence is to be reinforced - to make them aware of
the query.


> On Dec. 19, 2013, 3:47 a.m., Yuri Zelikov wrote:
> > src/org/waveprotocol/box/server/robots/agent/search/SolrRobot.java, line 199
> > <https://reviews.apache.org/r/16322/diff/2/?file=398901#file398901line199>
> >
> >     Do we really need the comment here?

See the reply to the review about the comment on "start == range.getEnd()".


> On Dec. 19, 2013, 3:47 a.m., Yuri Zelikov wrote:
> > src/org/waveprotocol/box/server/robots/agent/search/SolrRobot.java, line 289
> > <https://reviews.apache.org/r/16322/diff/2/?file=398901#file398901line289>
> >
> >     Remove comment

Add Java comment on com.google.wave.api.Blip.appendMarkup(String) to prevent reattempting
on that.


> On Dec. 19, 2013, 3:47 a.m., Yuri Zelikov wrote:
> > src/org/waveprotocol/box/server/robots/agent/search/SolrRobot.java, line 428
> > <https://reviews.apache.org/r/16322/diff/2/?file=398901#file398901line428>
> >
> >     Remove blank line

A blank line should make the code easier to read. For large blocks, blank line helps to group
statements.


- Frank


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16322/#review30627
-----------------------------------------------------------


On Dec. 21, 2013, 1:12 a.m., Frank R. wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16322/
> -----------------------------------------------------------
> 
> (Updated Dec. 21, 2013, 1:12 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/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.
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message