Re: [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/#review229
---



indra/newview/llbottomtray.h


The comment here says "Specifies buttons which can be hidden when bottom 
tray is shrunk."

Indeed, when you make the Viewer window narrower and narrower, after all 
buttons have reached their minimal size, the ones listed here (before your 
change, that's all bottom bar buttons except the speak button) will vanish one 
by one. (Making the window wider again will make them re-appear.)

Does listing the speak button here make it disappear in that case, too? If 
so, is that intended?



indra/newview/llbottomtray.cpp


Comments should be written for those reading the final code, not for those 
reading the diff. I.e. comment on what is done, maybe on how it is done and 
most important (if not obvious) why it's done. Do not comment on how it's done 
differently from earlier code or why the change was made. That would later only 
confuse those who aren't reading the old and new code side-by-side. A better 
place for comments on changes than the code are the jira issues, the 
"Description" field here on RB and last but no least the commit messages.

An exception is to be made when whole subsystems are changed in either very 
subtle or very fundamental ways (or both), so in-code comments pointing out the 
differences would help people already closely familiar with the old code to 
find their way around in the new one.

This comment here can probably just be removed.



indra/newview/llspeakbutton.cpp


Simplify your comments! No need to point out the obvious. (Here, that the 
"if" is some sort of check, that you added this check here (where else?) and 
that you do something dependent on the result of the check.)

// Only draw the speak button when voice is enabled

would capture the intent of the code perfectly and be quicker to grasp than

// Adding check here to see if ... if so then ...



indra/newview/llspeakbutton.cpp


Please don't remove the single empty line between the end of one method and 
the beginning of the next one.



indra/newview/skins/default/xui/en/menu_bottomtray.xml


Begin XML comments with just "", not "".



indra/newview/skins/default/xui/en/menu_bottomtray.xml


Indentation of name attribute is wrong. (Should be same as label and layout 
attributes.)



indra/newview/skins/default/xui/en/menu_bottomtray.xml


Make that
 label="Speak button (enables voice chat)"



indra/newview/skins/default/xui/en/menu_bottomtray.xml


More indentation strangeness.



indra/newview/skins/default/xui/en/menu_bottomtray.xml


Indentation of  is wrong. (Should be same level as 
preceding and following )


- 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
> 
>

___
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

Re: [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


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



indra/newview/skins/default/xui/en/menu_bottomtray.xml


"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


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
> 
>

___
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

Re: [opensource-dev] Review Request: VWR-24337: Possible crash on llassert_always(purge_list.size() >= entries_to_purge)

[Aleric Inglewood]

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

(Updated Jan. 21, 2011, 4:25 a.m.)


Review request for Viewer.


Changes
---

The new diff uses while(purge_list.size() < entries_to_purge), instead of a 
for() loop.


Summary
---

Just fixed the logic, so entries_to_purge won't become negative anymore, and 
the rest.


This addresses bug VWR-24337.
http://jira.secondlife.com/browse/VWR-24337


Diffs (updated)
-

  doc/contributions.txt 422f636c3343 
  indra/newview/lltexturecache.cpp 422f636c3343 

Diff: http://codereview.secondlife.com/r/93/diff


Testing
---


Thanks,

Aleric

___
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

Re: [opensource-dev] Review Request: VWR-24520: Don't use pkg_check_modules( ... QUIET ) on CMake < 2.8.2

[Boroondas Gupte]

> On Jan. 20, 2011, 11:33 p.m., Merov Linden wrote:
> > I'm advising the MM to merge in a test repo and do a full TC cycle on all 
> > platforms before merging though...

As far as I can see indra/cmake/FindLLQtWebkit.cmake only gets called by 
indra/cmake/WebKitLibPlugin.cmake (through find_package(LLQtWebkit REQUIRED 
QUIET)), and there the call only happens for STANDALONE. So I would be very 
surprised if this affects TC builds.


- Boroondas


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


On Jan. 17, 2011, 10:03 a.m., Boroondas Gupte wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/97/
> ---
> 
> (Updated Jan. 17, 2011, 10:03 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> Only use QUIET in pkg_check_modules() on CMake >=2.8.2 (where it's supported) 
> rather than already on CMake >=2.8.
> 
> 
> This addresses bug VWR-24520.
> http://jira.secondlife.com/browse/VWR-24520
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt 9e99b2c8fb28 
>   indra/cmake/FindLLQtWebkit.cmake 9e99b2c8fb28 
> 
> Diff: http://codereview.secondlife.com/r/97/diff
> 
> 
> Testing
> ---
> 
> Configured (standalone) without a .pgk file for libllqtwebkit on Linux with 
> CMake 2.8.1 and CMake 2.8.3. Output as expected.
> 
> Not tested:
> * CMake 2.8.2
> * system with a .pgk file for libllqtwebkit
> * non-standalone
> * Mac, Win
> 
> 
> Thanks,
> 
> Boroondas
> 
>

___
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

Re: [opensource-dev] Review Request: VWR-24312: Massively duplicated objects (part 2)

[Aleric Inglewood]

> On Jan. 14, 2011, 1:31 p.m., Boroondas Gupte wrote:
> > indra/llcommon/lllslconstants.h, line 184
> > 
> >
> > Yay for having type modifiers after the core type name. Makes them much 
> > easier to understand, especially when there are several cascaded ones. :-)
> 
> Aleric Inglewood wrote:
> Yeah, I'm strongly convinced that TYPE const is superior in anyway over 
> const TYPE.
> See http://www.xs4all.nl/~carlo17/cpp/const.qualifier.html for the 
> reasoning.
> In one line: all type qualifiers work to the left, it's best to PUT them 
> on the
> left so the whole type is logically uniform in it's construction. The 
> fact that
> you can legally type 'const TYPE' is just a historically grown misfortune.
>  
>

Of course I meant.. "All type qualifiers work to the left, so it's best to PUT 
them on the _right_ ...", as in:
TYPE QUALIFIER : The Qualifier changes the TYPE on it's left, so that what 
first was TYPE now becomes TYPE QUALIFIER.

[Where "QUALIFIER" is not just const, volatile, restrict, but also * and &.
The only exception where any qualifier works to the right is where 'const'
(volatile and restrict, but NOT * an &) works on a base type. Surely it needs
getting used when one changes style, but this one is so logical that already
after a single week you don't understand anymore how you were ever able to
use to previous format style.

While on the topic of coding style and types. The above is a good reason to
put * (and &) that are part of types *against* the TYPE they work on.
That way one can easily recognize the difference between the unary operator,
the binary operator and the type qualifier:

A* b = *c * d + e; // The type qualifier has no space on it's left (and
   // changes the type A), the binary operator (multiplication)
   // has spaces on both sides, while the unary operator
   // (dereference) works to it's right side and has no
   // space on the right.]

[[ Ie,

#include 

struct A { int i; };

int main()
{
  A const a[7] = { 0, 1, 2, 3, 4, 5, 6 };
  int const two = 2;

  int const* c = &two;
  int const  d = 3;
  A* const& e = &a[0];

  A* b = *c * d + e;

  std::cout << b->i << std::endl;
}

]]


- Aleric


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


On Jan. 16, 2011, 5:53 a.m., Aleric Inglewood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/81/
> ---
> 
> (Updated Jan. 16, 2011, 5:53 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> Turns out that most of my SNOW-800 patch was included in Viewer 2 (albeit 
> without crediting me).
> However, not everything was used and some more cleaning up was possible.
> 
> After this patch, and when compiling with optimization, there are no 
> duplicates left
> anymore that shouldn't be there in the first place: apart from the debug 
> stream
> iostream guard variable, there are several static variables with the same 
> name (r, r1,
> r2, etc) but that indeed actually different symbol objects. Then there are a 
> few
> constant POD arrays that are duplicated a hand full of times because they are
> accessed with a variable index (so optimizing them away is not possible). I 
> left them
> like that (although defining those as extern as well would have been more 
> consistent
> and not slower; in fact it would be faster theoretically because those arrays 
> could
> share the same cache page then). 
> 
> 
> This addresses bug VWR-24312.
> http://jira.secondlife.com/browse/VWR-24312
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt 422f636c3343 
>   indra/llcharacter/llanimationstates.cpp 422f636c3343 
>   indra/llcommon/llavatarconstants.h 422f636c3343 
>   indra/llcommon/lllslconstants.h 422f636c3343 
>   indra/llcommon/llmetricperformancetester.h 422f636c3343 
>   indra/llmath/llcamera.h 422f636c3343 
>   indra/llmath/llcamera.cpp 422f636c3343 
>   indra/newview/llviewerobject.cpp 422f636c3343 
>   indra/newview/llvoavatar.cpp 422f636c3343 
>   indra/newview/llvosky.h 422f636c3343 
>   indra/newview/llvosky.cpp 422f636c3343 
> 
> Diff: http://codereview.secondlife.com/r/81/diff
> 
> 
> Testing
> ---
> 
> Compiled with optimization and then running readelf on the executable to find 
> duplicated symbols.
> 
> 
> Thanks,
> 
> Aleric
> 
>

___
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

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

[Boroondas Gupte]

> On Jan. 21, 2011, 3:44 a.m., Boroondas Gupte wrote:
> > indra/newview/llspeakbutton.cpp, lines 67-70
> > 
> >
> > Please don't remove the single empty line between the end of one method 
> > and the beginning of the next one.

Oops, you didn't remove the line, it wasn't there before, just looks as if it 
was due to the diff view ... but can't hurt to add it.


- Boroondas


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


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
> 
>

___
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

Re: [opensource-dev] Review Request: VWR-24312: Massively duplicated objects (part 2)

[Aleric Inglewood]

> On Jan. 20, 2011, 10:57 p.m., Merov Linden wrote:
> > I fail to see how any of those changes "massively prevents object 
> > duplication". I don't disagree with any of them but I don't see much value 
> > in any either. Sure, there are different ways to skin a cat and some are 
> > better. Still, these changes will likely make upcoming merges conflict 
> > annoyingly and make the life of the maintainers (i.e. mine and Oz) horrid. 
> > It reminded me of this post by another (more famous) Open Source 
> > maintainer: http://tirania.org/blog/archive/2010/Dec-31.html

Hi Merov!

Firstly, I completely agree with the Open Source maintainer article, and I 
understand why this patch made you think it. However, being aware of this fact 
I don't think I made such changes to code that didn't already need change 
anyway. In fact, although certain styles used in the code are a horror in my 
eyes, I still don't change it-- even when I change those lines to fix a bug-- 
when the context around it all use the same style. A lot of style in the viewer 
code changes from file to file and even from function to function, but I try to 
preserve the style used at that point in the code.

But back to the subject at hand.

Surely you know most of the following already: I'm not writing this just for 
you but more in general for anyone reading this review (to produce some of the 
output below, I had to recompile the viewer several times, so I have some time 
to write this ;). Perhaps the two of us can talk about it on IRC, as well, if 
you want. In the line of the article that you linked to ;) ... you might just 
want to skip this long post to where it say **START HERE**.

An important concept to understand this patch is what is called Plain Old Data 
(POD). All built-in types are POD, as well as struct's that that only have POD 
members (in particular, have no user-defined constructor or destructor, for the 
gory definition read http://en.wikipedia.org/wiki/Plain_old_data_structure) 
other then the ones generated by the compiler. Why does this matter?

Well, consider we have two .cpp source files, source1.cpp and source2.cpp, both 
of which include the same header:

source1.cpp:

#include "header.h"
//...
int main() { }

source2.cpp:

#include "header.h"
//...

Further more, assume that the header defines a few constants:

header.h:

typedef int POD1;
struct POD2 { POD1 i1; POD1 i2; };

POD1 const const1 = 1;
POD2 const const2 = { 2, 2 };

struct nonPOD { POD1 i; nonPOD(int i_) : i(i_) { } };

nonPOD const const3(3);


Here, the struct nonPOD is not a POD type because it
has a user defined constructor. Note that
'nonPOD const const3 = { 3 }' would not compile,
as such initialization is only allowed for POD types.


First we'd compile both source files:

g++ -o source1.o -c source1.cpp
g++ -o source2.o -c source2.cpp

And then link them:

g++ -o test source1.o source2.o

We can use the utility nm(1) to dump the symbols
of each object file:

$ nm -C ./source1.o | egrep '(const[0-9]|POD)'
 r const1
0004 r const2
 b const3
 W nonPOD::nonPOD(int)

Here, just showing the interesting stuff (symbols containing 'const' or 'POD'),
we see all three constants, and the constructor of nonPOD.
Note that const1 and const2 have an 'r' in front of them while const3 has a 'b'.
This means respectively 'read-only section' and 'uninitialized section'.
The uninitialized section is not part of the final executable, it is space
allocated in memory at the moment an executable is started and subsequently
initialized, so it doesn't contribute the size of the executable but it
does cause more memory to be used during run time.

The other object file, source2.o has of course the exact
same output:

$ nm -C ./source2.o | egrep '(const[0-9]|POD)'
 r const1
0004 r const2
 b const3
 W nonPOD::nonPOD(int)

Finally we can look at the end result, the linked executable:

$ nm -C test | egrep '(const[0-9]|POD)'
0040071c r const1
00400724 r const1
00400720 r const2
00400728 r const2
00600b10 b const3
00600b14 b const3
004005d2 W nonPOD::nonPOD(int)

And as you see all constants appear twice in the executable.
Nothing we can do about that (until we compile with optimization,
see below).

Note that nm uses certain debug information.
If we strip the executable, like so;

$ strip test
$ nm -C test | egrep '(const[0-9]|POD)'
nm: test: no symbols

So clearly this doesn't give the REAL picture, but
good enough for this explanation.

If now we compile with optimization, which should
be the case that concerns us most, then the
results becomes:

$ g++ -O -o source1.o -c source1.cpp
$ g++ -O -o source2.o -c source2.cpp
$ g++ -o test source1.o source2.o
006009f0 b const3
006009f4 b const3

In other words, optimizing is able to
get rid of POD constants, but not of
non-POD constants: 

[opensource-dev] Link times revisited

[Jonathan Welch]

This is a follow up to my earlier Link Times message.
I just upgraded my system from 2Gb to 4Gb, though it only shows 3Gb as
being available (the joys of 32-bit Windows XP).

   1st2nd pass
CV 0:53   0:24  2Gb (CV = Cool Rainbow Viewer, based on v1.22)
  0:58  0:26  3Gb

SG 3:30  2:07  2Gb
  3:33  2:11  3Gb

V2  6:18  6:01  2Gb
  5:28  3:33  3Gb

When I was doing these timing tests, initially with 2Gb, I was also
monitoring free memory and did not see it get to an extremely low
value; perhaps data was being sent to the page file before that
happened, as there is a significant speed gain just by having 1 more
Gb.

-jonathan
___
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

[opensource-dev] FW: [JIRA] Commented: (STORM-236) Allow the "Speak" button to be removed, like other buttons

[WolfPup Lowenhar]

Please Help test this.

 

From: Oz Linden (JIRA) [mailto:no-re...@secondlife.com] 
Sent: Friday, January 21, 2011 10:57 AM
To: wolfpu...@earthlink.net
Subject: [JIRA] Commented: (STORM-236) Allow the "Speak" button to be removed, 
like other buttons

 

 

[ 
https://jira.secondlife.com/browse/STORM-236?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel
 

 &focusedCommentId=236477#comment-236477 ]

Oz Linden commented on STORM-236:
-

Test builds available at:
http://automated-builds-secondlife-com.s3.amazonaws.com/hg/repo/oz_project-2/rev/219418/index.html

> Allow the "Speak" button to be removed, like other buttons
> --
>
> Key: STORM-236
> URL: https://jira.secondlife.com/browse/STORM-236
> Project: Snowstorm
>  Issue Type: Story
> Environment: Irrelevant.
>Reporter: Mimika Oh
> Fix For: Product Backlog Proposals
>
> Attachments: cantspeakbutton.png, No Speak Button Voice Disabled.jpg, 
> Screen shot 2010-03-31 at 19.24.55.png, Speak Button Voice Enabled.jpg
>
>  Time Spent: 2 days, 1 hour
>  Remaining Estimate: 7 hours
>
> As a user who does not use Voice, I would like to reclaim the bottom-bar 
> space occupied by the disabled "Speak" buttons.
> The bottom bar of the viewer has buttons which can be enabled and disabled by 
> right-clicking the bar. Except the Speak button, which remains even when 
> Voice is totally disabled, and can't be removed.
> Please allow it to be removed.  When voice is disabled, it is just wasting 
> space.

--
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira



  _  

No virus found in this message.
Checked by AVG - www.avg.com
Version: 10.0.1191 / Virus Database: 1435/3393 - Release Date: 01/20/11

___
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

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

[Alexandrea Fride]

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


Perfect just the naming in menu should be better "Speak button (enables voice 
chat)" like boroondas sayd :) (and mayby the styling fixed or so but patch 
works as exspected)

tested patch and test build both worked as exspected

nice job

- Alexandrea


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
> 
>

___
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

Re: [opensource-dev] FW: [JIRA] Commented: (STORM-236) Allow the "Speak" button to be removed, like other buttons

[Trilo Byte]

Works great in the Mac client.  Sounds nitpicky, but I'd add a space after 
"Speak Button" and before "(Voice Enabled" in the menu


On Jan 21, 2011, at 8:41 AM, WolfPup Lowenhar wrote:

> Please Help test this.
>  
> From: Oz Linden (JIRA) [mailto:no-re...@secondlife.com] 
> Sent: Friday, January 21, 2011 10:57 AM
> To: wolfpu...@earthlink.net
> Subject: [JIRA] Commented: (STORM-236) Allow the "Speak" button to be 
> removed, like other buttons
>  
>  
> [ 
> https://jira.secondlife.com/browse/STORM-236?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=236477#comment-236477
>  ]
> 
> Oz Linden commented on STORM-236:
> -
> 
> Test builds available at:
> http://automated-builds-secondlife-com.s3.amazonaws.com/hg/repo/oz_project-2/rev/219418/index.html
> 
> > Allow the "Speak" button to be removed, like other buttons
> > --
> >
> > Key: STORM-236
> > URL: https://jira.secondlife.com/browse/STORM-236
> > Project: Snowstorm
> >  Issue Type: Story
> > Environment: Irrelevant.
> >Reporter: Mimika Oh
> > Fix For: Product Backlog Proposals
> >
> > Attachments: cantspeakbutton.png, No Speak Button Voice 
> > Disabled.jpg, Screen shot 2010-03-31 at 19.24.55.png, Speak Button Voice 
> > Enabled.jpg
> >
> >  Time Spent: 2 days, 1 hour
> >  Remaining Estimate: 7 hours
> >
> > As a user who does not use Voice, I would like to reclaim the bottom-bar 
> > space occupied by the disabled "Speak" buttons.
> > The bottom bar of the viewer has buttons which can be enabled and disabled 
> > by right-clicking the bar. Except the Speak button, which remains even when 
> > Voice is totally disabled, and can't be removed.
> > Please allow it to be removed.  When voice is disabled, it is just wasting 
> > space.
> 
> --
> This message is automatically generated by JIRA.
> -
> For more information on JIRA, see: http://www.atlassian.com/software/jira
> 
>
> 
> No virus found in this message.
> Checked by AVG - www.avg.com
> Version: 10.0.1191 / Virus Database: 1435/3393 - Release Date: 01/20/11
> 
> ___
> 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

___
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

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

[Opensource Obscure]

It seems it works as expected on Linux after some testing (2.6.0 (219418)).
The UI works fine, but I didn't use voice during the tests.

I don't know if this is by design, but I'll add that:
- when you disable Voice, the Speak Button correctly disappears
- this doesn't happen if you disable the whole SL audio system


Thanks for this feature!
Opensource Obscure


On Fri, Jan 21, 2011 at 18:06, Alexandrea Fride  wrote:

>This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/113/
>
> Perfect just the naming in menu should be better "Speak button (enables voice 
> chat)" like boroondas sayd :) (and mayby the styling fixed or so but patch 
> works as exspected)
>
> tested patch and test build both worked as exspected
>
> nice job
>
>
> - Alexandrea
>
> On January 20th, 2011, 6:37 p.m., Wolfpup Lowenhar wrote:
>   Review request for Viewer.
> By Wolfpup Lowenhar.
>
> *Updated Jan. 20, 2011, 6:37 p.m.*
> Description
>
> This allows the Speak Button to auto-hide for those that do not use Voice at 
> all.
>
>   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.
>
>   *Bugs: * 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)
>
> View Diff 
>
> ___
> 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
>



-- 
*Opensource Obscure*

Twitter  [EN] -
Twitter [IT]
- Blog  [IT] -
YouTube
 - 
Photos
 - Second 
Life
___
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

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

[Merov Linden]

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



indra/newview/llbottomtray.cpp


Unnecessary comment. Please delete.



indra/newview/llbottomtray.cpp


Unnecessary comment. Please delete.



indra/newview/llbottomtray.cpp


What Boroondas said. Please delete that comment.



indra/newview/llbottomtray.cpp


Unnecessary comment.



indra/newview/llbottomtray.cpp


Unnecessary comment.



indra/newview/skins/default/xui/en/menu_bottomtray.xml


Typos: change "selction" to "selection", change "visable" to "visible"


- Merov


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
> 
>

___
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

Re: [opensource-dev] Review Request: VWR-24337: Possible crash on llassert_always(purge_list.size() >= entries_to_purge)

[Merov Linden]

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

Ship it!


Better! You may want to reword a bit the comment above the "while()" statement 
though.

- Merov


On Jan. 21, 2011, 4:25 a.m., Aleric Inglewood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/93/
> ---
> 
> (Updated Jan. 21, 2011, 4:25 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> Just fixed the logic, so entries_to_purge won't become negative anymore, and 
> the rest.
> 
> 
> This addresses bug VWR-24337.
> http://jira.secondlife.com/browse/VWR-24337
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt 422f636c3343 
>   indra/newview/lltexturecache.cpp 422f636c3343 
> 
> Diff: http://codereview.secondlife.com/r/93/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Aleric
> 
>

___
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

[opensource-dev] line ordering in contributions.txt

[Oz Linden (Scott Lawrence)]

> Actually, because I use changes to this file to generate some of our metrics 
> for open source contributions, I'd prefer that any existing order be 
> preserved: that is, add lines as needed (and adding them in order is nice), 
> but do not reorder existing lines please.

I've fixed my script - it no longer cares what order those lines are in 
(and generates slightly more accurate stats even when they are not 
reordered, since if the same issue is listed under multiple contributors 
it is now counted only once).


___
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

Re: [opensource-dev] Review Request: STORM-2 As a User, I want to set my own default views with specific UI layout so I can tailor my Viewer experience to the activities I'm most interested in.

[Oz Linden]

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


decided this morning that this change will not be integrated yet, so closing 
this review pending completing the work

- Oz


On Jan. 17, 2011, 1:24 p.m., Andrew ProductEngine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/98/
> ---
> 
> (Updated Jan. 17, 2011, 1:24 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> First pass of saving of UI state into files.
> 
> Implemeted saving of floaters and bottombar buttons state into a named 
> layout. This layout is saved on disk into per-user folder. The folder with 
> layout contains LLSD files.
> 
> Currently the following features were implemeted:
> - Saving of bottomtray buttons order and visibility.
> - Saving floaters rect and visibility info (multiple instances of the same 
> floater are supported)
> - Saving of sidetray tabs state (docked/undocked).
> - UI for the feature.
> 
> TODO:
> - Add saving dock state for dockable floaters.
> - Add LL-created default layouts and support for them.
> - Refactor system of updating lists - get rid of ugly static dirty and it's 
> check in draw.
> - Add saving of sidetray open/closed state, open tab, etc.
> 
> 
> This addresses bug STORM-2.
> http://jira.secondlife.com/browse/STORM-2
> 
> 
> Diffs
> -
> 
>   indra/llui/llfloater.h 422f636c3343 
>   indra/llui/llfloaterreg.h 422f636c3343 
>   indra/newview/CMakeLists.txt 422f636c3343 
>   indra/newview/llbottomtray.h 422f636c3343 
>   indra/newview/llbottomtray.cpp 422f636c3343 
>   indra/newview/llfloaterlayouts.h PRE-CREATION 
>   indra/newview/llfloaterlayouts.cpp PRE-CREATION 
>   indra/newview/llsidetray.h 422f636c3343 
>   indra/newview/llsidetray.cpp 422f636c3343 
>   indra/newview/llviewerfloaterreg.cpp 422f636c3343 
>   indra/newview/llviewermenu.cpp 422f636c3343 
>   indra/newview/skins/default/xui/en/floater_layouts.xml PRE-CREATION 
>   indra/newview/skins/default/xui/en/floater_save_layout.xml PRE-CREATION 
>   indra/newview/skins/default/xui/en/menu_viewer.xml 422f636c3343 
>   indra/newview/skins/default/xui/en/notifications.xml 422f636c3343 
> 
> Diff: http://codereview.secondlife.com/r/98/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew
> 
>

___
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

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

[Cron Stardust]

> On Jan. 21, 2011, 3:44 a.m., Boroondas Gupte wrote:
> > indra/newview/skins/default/xui/en/menu_bottomtray.xml, lines 11-13
> > 
> >
> > Begin XML comments with just "", not 
> > "".

Typically SGML/XML comments are supposed to start with "" (note the post/pre-fixed whitespace) - however, 
http://www.w3.org/TR/2008/REC-xml-20081126/#sec-comments does not require the 
space in the syntax, it only demonstrates it.


- Cron


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


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
> 
>

___
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

Re: [opensource-dev] Review Request: STORM-2 As a User, I want to set my own default views with specific UI layout so I can tailor my Viewer experience to the activities I'm most interested in.

[Merov Linden]

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

Ship it!


Good. Adding my stamp of approval.

- Merov


On Jan. 17, 2011, 1:24 p.m., Andrew ProductEngine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/98/
> ---
> 
> (Updated Jan. 17, 2011, 1:24 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> First pass of saving of UI state into files.
> 
> Implemeted saving of floaters and bottombar buttons state into a named 
> layout. This layout is saved on disk into per-user folder. The folder with 
> layout contains LLSD files.
> 
> Currently the following features were implemeted:
> - Saving of bottomtray buttons order and visibility.
> - Saving floaters rect and visibility info (multiple instances of the same 
> floater are supported)
> - Saving of sidetray tabs state (docked/undocked).
> - UI for the feature.
> 
> TODO:
> - Add saving dock state for dockable floaters.
> - Add LL-created default layouts and support for them.
> - Refactor system of updating lists - get rid of ugly static dirty and it's 
> check in draw.
> - Add saving of sidetray open/closed state, open tab, etc.
> 
> 
> This addresses bug STORM-2.
> http://jira.secondlife.com/browse/STORM-2
> 
> 
> Diffs
> -
> 
>   indra/llui/llfloater.h 422f636c3343 
>   indra/llui/llfloaterreg.h 422f636c3343 
>   indra/newview/CMakeLists.txt 422f636c3343 
>   indra/newview/llbottomtray.h 422f636c3343 
>   indra/newview/llbottomtray.cpp 422f636c3343 
>   indra/newview/llfloaterlayouts.h PRE-CREATION 
>   indra/newview/llfloaterlayouts.cpp PRE-CREATION 
>   indra/newview/llsidetray.h 422f636c3343 
>   indra/newview/llsidetray.cpp 422f636c3343 
>   indra/newview/llviewerfloaterreg.cpp 422f636c3343 
>   indra/newview/llviewermenu.cpp 422f636c3343 
>   indra/newview/skins/default/xui/en/floater_layouts.xml PRE-CREATION 
>   indra/newview/skins/default/xui/en/floater_save_layout.xml PRE-CREATION 
>   indra/newview/skins/default/xui/en/menu_viewer.xml 422f636c3343 
>   indra/newview/skins/default/xui/en/notifications.xml 422f636c3343 
> 
> Diff: http://codereview.secondlife.com/r/98/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew
> 
>

___
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

Re: [opensource-dev] Review Request: VWR-24321: Validate textures starting with 00 too.

[Merov Linden]

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

Ship it!


Ok, I read the code in more details and I'm indeed convinced now that this is 
correct. Ship it!

- Merov


On Jan. 14, 2011, 1:02 p.m., Aleric Inglewood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/90/
> ---
> 
> (Updated Jan. 14, 2011, 1:02 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> Trivial patch, just removes stupidity.
> 
> 
> This addresses bug VWR-24321.
> http://jira.secondlife.com/browse/VWR-24321
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt b0bd26c5638a 
>   indra/newview/lltexturecache.cpp b0bd26c5638a 
> 
> Diff: http://codereview.secondlife.com/r/90/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Aleric
> 
>

___
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

Re: [opensource-dev] Review Request: Help Needed to debug small problem with code for STORM-236.

[Merov Linden]

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

Ship it!


Good code wise. Please keep comments short and clean.


indra/newview/llspeakbutton.cpp


Prefer: // Draw the voice button only if voice chat is enabled



indra/newview/skins/default/xui/en/menu_bottomtray.xml


Please clean up typos (selction, visable, )


- Merov


On Jan. 20, 2011, 9:29 a.m., Wolfpup Lowenhar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/112/
> ---
> 
> (Updated Jan. 20, 2011, 9:29 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> This is actually a request for for help to debug what I have done so far to 
> get this working.
> 1. In the current diff the button dose actualy hide and show according to if 
> Voice is enabled or not.
> 2. i have two problems at this point.
>a. the space taken up by the speak button is not freed up
>b. when the speak button is shown after being hidden there is an error 
> message saying no space avaiable
>   (I think this is related to the fact that the space is not freed to 
> begin with).
> 
> 
> This addresses bug STORM-236.
> http://jira.secondlife.com/browse/STORM-236
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt 40d0806e9800 
>   indra/newview/llbottomtray.cpp 40d0806e9800 
>   indra/newview/llspeakbutton.cpp 40d0806e9800 
>   indra/newview/skins/default/xui/en/menu_bottomtray.xml 40d0806e9800 
> 
> Diff: http://codereview.secondlife.com/r/112/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Wolfpup
> 
>

___
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

[opensource-dev] google-breakpad VS2010 build project. Linden Dev Help Please.

[Nicky Perian]

I am trying to prove a no harm done 2005 build first. Then on to 2010.
the build-cmd.sh script references "build_sln" for several entries. 
Example: build_sln src/client/windows/breakpad_client.sln "release|win32" 
exception_handler

These all fail when I try them. Is build_sln common to the LL build system and 
not included in 3p-google-breakpad source?

The sln files build with  cd src/client/windows
../../tools/gyp/gyp -G msvs_version=2005

Is this just and automation thingy? 
Can I just build with debug and release or do I need build_sln's magic?
Nicky


  ___
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

Re: [opensource-dev] Review Request: make PREHASH variables char const* const

[Merov Linden]

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

Ship it!


Not a fan of widespread cleanup changes as I said in a previous review, but if 
this is getting rid of compilation errors on some machines, I'm all for it. 

- Merov


On Jan. 20, 2011, 2:57 p.m., Boroondas Gupte wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/100/
> ---
> 
> (Updated Jan. 20, 2011, 2:57 p.m.)
> 
> 
> Review request for Viewer and Seth ProductEngine.
> 
> 
> Summary
> ---
> 
> For the reason for this change, see 
> https://jira.secondlife.com/browse/VWR-24487 and 
> https://jira.secondlife.com/browse/VWR-24522
> 
> What I did:
> In indra/llmessage/message_prehash.(h|cpp), I turned everything into constant 
> pointers to constants by search/replace. Then I tried to compile and added 
> const qualifiers in dependent code as needed to stop the compiler complaining.
> 
> Note that comments in indra/llmessage/message_prehash.(h|cpp) say these files 
> have been generated from the message template. If this generation wasn't a 
> one-off thing, the generating code must be changed, too, so it won't override 
> this change here when the generation happens the next time.
> 
> 
> This addresses bug VWR-24487.
> http://jira.secondlife.com/browse/VWR-24487
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt fc7e5dcf3059 
>   indra/llmessage/message_prehash.h fc7e5dcf3059 
>   indra/llmessage/message_prehash.cpp fc7e5dcf3059 
>   indra/llprimitive/llprimitive.h fc7e5dcf3059 
>   indra/llprimitive/llprimitive.cpp fc7e5dcf3059 
>   indra/llprimitive/llvolumemessage.h fc7e5dcf3059 
>   indra/llprimitive/llvolumemessage.cpp fc7e5dcf3059 
>   indra/llui/tests/llurlentry_stub.cpp fc7e5dcf3059 
>   indra/newview/tests/llremoteparcelrequest_test.cpp fc7e5dcf3059 
> 
> Diff: http://codereview.secondlife.com/r/100/diff
> 
> 
> Testing
> ---
> 
> Compiled (standalone, 64bit Linux) with and without LL_TESTS.
> Started the viewer, logged in, walked and flew around a bit. Everything seems 
> to work.
> 
> 
> Locally set _PREHASH_AgentData and _PREHASH_AgentID to (char const*)0x1 in 
> indra/llui/tests/llurlentry_stub.cpp and 
> indra/newview/tests/llremoteparcelrequest_test.cpp to verify they actually 
> are never dereferenced, even when not NULL, so that using NULL pointers 
> instead of place holder data won't change what code paths gets tested. Both 
> tests binaries executed without crashes and all the contained tests passed.
> 
> 
> Thanks,
> 
> Boroondas
> 
>

___
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