> On Dec. 18, 2013, 7:47 p.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
> 
> Frank R. wrote:
>     A blank line should make the code easier to read. For large blocks, blank 
> line helps to group statements.

Blank line is OK when you want to group lines of code into several logic 
statements, however, there's no need to add blank line before end of block. In 
most cases when there are a lot of blank lines it means that the method does 
several things and need to be refactored into several smaller methods. Blank 
lines make methods longer, and the longer the method is - the harder it is to 
understand what it does. So, please put blank lines only when absolutely 
necessary and of there are more than a couple - consider a refactor.


- Yuri


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


On Dec. 21, 2013, 4:38 p.m., Frank R. wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16322/
> -----------------------------------------------------------
> 
> (Updated Dec. 21, 2013, 4:38 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
> -----
> 
>   .gitignore fe1dbc9 
>   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 
>   
> src/org/waveprotocol/wave/client/doodad/selection/SelectionAnnotationHandler.java
>  158876a 
> 
> 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.
> 
>

Reply via email to