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

Ship it!


Functionally, it looks good...just one minor comment about structure


indra/newview/llappviewer.cpp
<http://codereview.secondlife.com/r/607/#comment1161>

    Structurally, it seems like this would be cleaner if opening the file, 
writing out the marke version, and closing it all happened in one function.  
Otherwise, you have this constraint that the file needs to be opened before 
being passed into recordMarkerVersion and closed afterwards.
    
    Just picking a nit, sorry.


- Richard Nelson


On Nov. 2, 2012, 1:55 p.m., Oz Linden wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/607/
> -----------------------------------------------------------
> 
> (Updated Nov. 2, 2012, 1:55 p.m.)
> 
> 
> Review request for Viewer and Callum Prentice.
> 
> 
> Description
> -------
> 
> In all the marker files used to detect how the viewer run terminates, record 
> the version.  When checking the results, report errors only if the current 
> version matches the version in the file.  This prevents errors in one version 
> from being reported against the subsequent version.
> 
> 
> This addresses bug storm-1850.
>     http://jira.secondlife.com/browse/storm-1850
> 
> 
> Diffs
> -----
> 
>   indra/newview/llappviewer.h 3d35a13561fc 
>   indra/newview/llappviewer.cpp 3d35a13561fc 
> 
> Diff: http://codereview.secondlife.com/r/607/diff/
> 
> 
> Testing
> -------
> 
> Several simulated crashes both of the modified and unmodified viewers, and 
> some in which the marker file was modified manually to simulate different 
> viewers. Launched the new viewer after different crashes (and normal exits) 
> and confirmed (using logging temporarily added for that purpose) that the 
> reported last exec event was correct - and is always reported as Normal if 
> the previous version and the running version were not the same.
> 
> 
> Thanks,
> 
> Oz Linden
> 
>

_______________________________________________
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Reply via email to