[opensource-dev] Review Request: STORM-236 Actual Code Review

[Boroondas Gupte]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/113/#review230
-----------------------------------------------------------


Looks like RB ate some of my comments in the review above. (Maybe because the quoted code sections overlapped.)


indra/newview/llspeakbutton.cpp
<http://codereview.secondlife.com/r/113/#comment167>

    The code here should be indented the same as the comment.



indra/newview/skins/default/xui/en/menu_bottomtray.xml
<http://codereview.secondlife.com/r/113/#comment169>

    "section"? "selection"? Please check the spelling.
    
    Also, the comment here confused me. I think what you meant to say is: "The Speak Button is visible if and only if Voice Chat is enabled. Thus, to toggle the button's visibility, we enable or disable Voice Chat accordingly."



indra/newview/skins/default/xui/en/menu_bottomtray.xml
<http://codereview.secondlife.com/r/113/#comment165>

    Trailing whitespace.
    (Didn't RB highlight those in red, before? Doesn't seem to do that anymore in the diff view, only in quoted code on the review.)


- Boroondas


On Jan. 20, 2011, 6:37 p.m., Wolfpup Lowenhar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/113/
> -----------------------------------------------------------
> 
> (Updated Jan. 20, 2011, 6:37 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> -------
> 
> This allows the Speak Button to auto-hide for those that do not use Voice at all.
> 
> 
> This addresses bug STORM-236.
>     http://jira.secondlife.com/browse/STORM-236
> 
> 
> Diffs
> -----
> 
>   doc/contributions.txt 9c7d543fd15d 
>   indra/newview/llbottomtray.h 9c7d543fd15d 
>   indra/newview/llbottomtray.cpp 9c7d543fd15d 
>   indra/newview/llspeakbutton.cpp 9c7d543fd15d 
>   indra/newview/skins/default/xui/en/menu_bottomtray.xml 9c7d543fd15d 
> 
> Diff: http://codereview.secondlife.com/r/113/diff
> 
> 
> Testing
> -------
> 
> Built locally and did the following:
> 1 Verified that when Voice is toggled via the preference panel the Speak Button auto hid/showed.
> 2 Verified that drag and drop functionality of the Speak Button was not affected.
> 3 Went to a non-Voice area with Voice active and verified that button was still there but grayed out.
> 
> 
> Thanks,
> 
> Wolfpup
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.secondlife.com/pipermail/opensource-dev/attachments/20110121/904b2ecc/attachment.htm