[opensource-dev] Review Request: CHOP-619 fix for pre-compiled headers issue

[Boroondas Gupte]

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



indra/llxuixml/llxuiparser.h
<http://codereview.secondlife.com/r/289/#comment630>

    Any reason to keep this commented out instead of removing it completely?



indra/newview/llagent.cpp
<http://codereview.secondlife.com/r/289/#comment631>

    I know it's best practice to always set pointers to NULL after deleting, but does that also apply in the destructor? If so, maybe it should also be done for mMouselookModeInSignal and mMouselookModeOutSignal.



indra/newview/llagent.cpp
<http://codereview.secondlife.com/r/289/#comment632>

    While we're editing around here, should this comment still be kept?



indra/newview/llviewerregion.h
<http://codereview.secondlife.com/r/289/#comment635>

    Should probably be a const method.



indra/newview/llviewerregion.cpp
<http://codereview.secondlife.com/r/289/#comment633>

    Comments referring to "I" should probably be signed, so one knows who's speaking.



indra/newview/llviewerregion.cpp
<http://codereview.secondlife.com/r/289/#comment634>

    Fix indentation.



indra/newview/llviewerregion.cpp
<http://codereview.secondlife.com/r/289/#comment636>

    Consider breaking this long line into several lines.



indra/newview/llviewerregion.cpp
<http://codereview.secondlife.com/r/289/#comment637>

    Please place spaces around binary operator* .



indra/newview/llviewerregion.cpp
<http://codereview.secondlife.com/r/289/#comment638>

    Consider using pre-increments instead of post-increments.



indra/newview/llviewerwindow.h
<http://codereview.secondlife.com/r/289/#comment639>

    Any reason to keep these commented out rather than removing them completely?


- Boroondas


On May 3, 2011, 6:22 p.m., Brad Kittenbrink wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/289/
> -----------------------------------------------------------
> 
> (Updated May 3, 2011, 6:22 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> -------
> 
> Fix for error using precompiled headers with vs2010 and incredibuild 3.60 beta2.
> 
> We'll use this opportunity to remove all the special cases we had for working around this problem.
> 
> 
> This addresses bug CHOP-616.
>     http://jira.secondlife.com/browse/CHOP-616
> 
> 
> Diffs
> -----
> 
>   indra/llcommon/llapp.cpp UNKNOWN 
>   indra/llui/llbutton.h UNKNOWN 
>   indra/llui/llfloaterreg.h UNKNOWN 
>   indra/llui/llfocusmgr.h UNKNOWN 
>   indra/llui/llfocusmgr.cpp UNKNOWN 
>   indra/llui/lliconctrl.h UNKNOWN 
>   indra/llui/lllineeditor.h UNKNOWN 
>   indra/llui/llloadingindicator.h UNKNOWN 
>   indra/llui/llmultislider.cpp UNKNOWN 
>   indra/llui/llpanel.h UNKNOWN 
>   indra/llui/llprogressbar.h UNKNOWN 
>   indra/llui/llprogressbar.cpp UNKNOWN 
>   indra/llui/llslider.h UNKNOWN 
>   indra/llui/llstyle.h UNKNOWN 
>   indra/llui/llstyle.cpp UNKNOWN 
>   indra/llui/lltransutil.cpp UNKNOWN 
>   indra/llui/llui.h UNKNOWN 
>   indra/llui/llview.h UNKNOWN 
>   indra/llui/llviewborder.cpp UNKNOWN 
>   indra/llui/llwindowshade.h UNKNOWN 
>   indra/llxuixml/lltrans.h UNKNOWN 
>   indra/llxuixml/lltrans.cpp UNKNOWN 
>   indra/llxuixml/llxuiparser.h UNKNOWN 
>   indra/newview/llagent.h UNKNOWN 
>   indra/newview/llagent.cpp UNKNOWN 
>   indra/newview/llappviewer.cpp UNKNOWN 
>   indra/newview/lleventnotifier.h UNKNOWN 
>   indra/newview/llfloaterland.h UNKNOWN 
>   indra/newview/llfloaterland.cpp UNKNOWN 
>   indra/newview/llfloatersnapshot.cpp UNKNOWN 
>   indra/newview/llfolderviewitem.h UNKNOWN 
>   indra/newview/lllocationhistory.h UNKNOWN 
>   indra/newview/lloutputmonitorctrl.h UNKNOWN 
>   indra/newview/llpanelavatar.cpp UNKNOWN 
>   indra/newview/llpanelgroupgeneral.cpp UNKNOWN 
>   indra/newview/llpanelgrouproles.cpp UNKNOWN 
>   indra/newview/llpreviewgesture.cpp UNKNOWN 
>   indra/newview/llsidepaneliteminfo.cpp UNKNOWN 
>   indra/newview/lltooldraganddrop.cpp UNKNOWN 
>   indra/newview/llviewerchat.cpp UNKNOWN 
>   indra/newview/llviewerkeyboard.h UNKNOWN 
>   indra/newview/llviewermenu.cpp UNKNOWN 
>   indra/newview/llviewerparcelmgr.cpp UNKNOWN 
>   indra/newview/llviewerprecompiledheaders.h UNKNOWN 
>   indra/newview/llviewerregion.h UNKNOWN 
>   indra/newview/llviewerregion.cpp UNKNOWN 
>   indra/newview/llviewertexturelist.h UNKNOWN 
>   indra/newview/llviewerwindow.h UNKNOWN 
>   indra/newview/llviewerwindow.cpp UNKNOWN 
>   indra/newview/llvoavatar.cpp UNKNOWN 
>   indra/newview/llvotree.cpp UNKNOWN 
>   indra/newview/llvovolume.cpp UNKNOWN 
>   indra/newview/llworld.cpp UNKNOWN 
>   indra/newview/tests/llremoteparcelrequest_test.cpp UNKNOWN 
>   indra/newview/tests/llviewerhelputil_test.cpp UNKNOWN 
> 
> Diff: http://codereview.secondlife.com/r/289/diff
> 
> 
> Testing
> -------
> 
> I tested builds personally on windows and linux although the cmake changes can only affect windows.
> 
> The changes are such that testing the viewer itself is completely unnecessary.  If it builds, it's good.
> 
> 
> Thanks,
> 
> Brad
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.secondlife.com/pipermail/opensource-dev/attachments/20110504/42fcc8af/attachment-0001.htm