> On March 29, 2012, 5:28 a.m., Vaalith Jinn wrote: > > indra/newview/lllocalbitmaps.h, line 42 > > <http://codereview.secondlife.com/r/566/diff/1/?file=7720#file7720line42> > > > > For clarity's sake i kept private enums block below where it's > > currently at. > > > > If i were to add an enum value in here i'd have to either move the > > whole block up to keep consistency or break consistency and write in a > > single enum below the main public block.. > > > > Neither is a really pretty solution as far as readability is concerned. > > > > Is it really worth it?
I think it'd be worth it, as it'd make the calling code much more readable. See http://doc.qt.nokia.com/qq/qq13-apis.html#thebooleanparametertrap Note that in the example at hand, the function will only be called with boolean literals, not boolean variables, so we don't have the problem of forcing additional if-else code onto the caller. Also, half if not all of the lengthy comment above the one place where you call the function with "true" could be saved if a well-named constant or enum would be passed instead. - Boroondas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/566/#review1199 ----------------------------------------------------------- On March 30, 2012, 4:06 p.m., Vaalith Jinn wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://codereview.secondlife.com/r/566/ > ----------------------------------------------------------- > > (Updated March 30, 2012, 4:06 p.m.) > > > Review request for Viewer, Callum Linden and Richard Nelson. > > > Description > ------- > > Local Bitmaps is a mechanism to locally load images into the viewer, track > them and optionally (per each image) > have it check if the image has been overwritten locally and if so - update it > in the viewer and inworld. > > This change affects the texture picker - adding radio checks that let the > user choose between the "Inventory" (regular inventory) and "Local" tabs > (list of locally added files). > > *** DO NOT USE THIS YET *** > do not implement for live viewers until fully reviewed. > > > This addresses bug STORM-64. > http://jira.secondlife.com/browse/STORM-64 > > > Diffs > ----- > > doc/contributions.txt 96a53bafcd1c > indra/newview/CMakeLists.txt 96a53bafcd1c > indra/newview/lllocalbitmaps.h PRE-CREATION > indra/newview/lllocalbitmaps.cpp PRE-CREATION > indra/newview/lltexturectrl.h 96a53bafcd1c > indra/newview/lltexturectrl.cpp 96a53bafcd1c > indra/newview/llviewertexturelist.h 96a53bafcd1c > indra/newview/llwearable.h 96a53bafcd1c > indra/newview/llwearable.cpp 96a53bafcd1c > indra/newview/skins/default/xui/en/floater_texture_ctrl.xml 96a53bafcd1c > > Diff: http://codereview.secondlife.com/r/566/diff/diff > > > Testing > ------- > > > Thanks, > > Vaalith Jinn > >
_______________________________________________ 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
