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 11:00:34 GMT

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



src/org/waveprotocol/box/server/robots/agent/search/SolrRobot.java
<https://reviews.apache.org/r/16322/#comment58949>

    Does this comply with the code style reinforced here?
    
    http://maven.apache.org/developers/conventions/code.html
    
    I don't think this is respected.
    Blocks: Always enclose with a new line brace.
    
    I used to have http://maven.apache.org/plugins/maven-checkstyle-plugin/
    
    I don't think some rules make sense at all.
    
    Anyway, where can I read about the rules? Thanks.
    
    p.s. sorry for the delay in response. I'm still a learner to the review site.



src/org/waveprotocol/box/server/robots/agent/search/SolrRobot.java
<https://reviews.apache.org/r/16322/#comment58919>

    Thanks for showing me the constants. I'll change their scope from private to public.
    
    Can you explain more what you mean by "these"? Thanks



src/org/waveprotocol/box/server/robots/agent/search/SolrRobot.java
<https://reviews.apache.org/r/16322/#comment58946>

    Yes. This method is only invoked for such case. It's extracted only to make it easy to
read the comments where it's invoked.



src/org/waveprotocol/box/server/robots/agent/search/SolrRobot.java
<https://reviews.apache.org/r/16322/#comment58947>

    It's feature to be implemented. If the TODO was to be removed, who else in the world would
know that? The comments are here for a reason - at least, you'll know how you can help to
improve the code around them, or, why they are what they are, and what problem other should
avoid when they collaborate with me.
    
    I'll make my Java comments more readable. Please give me feedback when you've actually
read them. Thanks.



src/org/waveprotocol/box/server/robots/agent/search/SolrRobot.java
<https://reviews.apache.org/r/16322/#comment58948>

    Would it be more readable?
    BundledAnnotation.listOf(Annotation.BACKGROUND_COLOR, ERROR_BACKGROUND_COLOR,
                  Annotation.COLOR, ERROR_COLOR), message);
    
    Why isn't "style/backgroundColor" defined in a single place, com.google.wave.api.Annotation.BACKGROUND_COLOR?
    
    See org.waveprotocol.wave.client.editor.content.misc.StyleAnnotationHandler.KEYS, and
org.waveprotocol.wave.model.richtext.RichTextMutationBuilder.STYLE_KEY_BG_COLOR.


- Frank R.


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