VWR-24100 Settings.xml: redundant entries and unnecessary tag
Review Request #18 - Created Dec. 14, 2010 and submitted
Jonathan Yap | Reviewers | ||
2.5 | viewer | ||
vwr-24100 | |||
None | viewer-development |
I wrote two programs that use settings.xml to produce this massive table: http://wiki.secondlife.com/wiki/Debug_Settings While doing this I found 4 places with duplicate entries and 1 entry that is repeated 4 times. There is also a pair of unnecessary tags. Having these cleaned out would make running my program, and thus updating the wiki table, easier. I've erased all but the last entry for these redundant debug settings.
looks good
any code or xml clean up is a good thing i think.
Posted (Dec. 16, 2010, 8:04 a.m.)
..testing the review system (hi mom!)..
-
indra/newview/app_settings/settings.xml (Diff revision 1) -
Considering the setting name (CacheLocationTopFolder), isn't the deleted Comment string better than the one you left in? Thus, Controls the top folder location of the the local disk cache, rather than Controls the location of the local disk cache. Diff is ok with me, I just wondered if you saw that it wasn't an exact duplicate.
-
indra/newview/app_settings/settings.xml (Diff revision 1) -
This deletes the entry with 60 seconds between printing the FPS and leaves the one that has 10 seconds between printing. Which is the correct one?
-
indra/newview/app_settings/settings.xml (Diff revision 1) -
This really looks wrong. The first OutBandwidth has Comment that has nothing to do with OutBandwidth and a Type Boolean. The one that was left in seem to be the real OutBandwidth looking at the Comment, and has Type F32. My guess is that the deleted entry should not have been deleted, but that the key of this entry is wrong, and that the correct way to fix this is to change the key into whatever it was intended to be.
Posted (Dec. 16, 2010, 10:18 a.m.)
-
indra/newview/app_settings/settings.xml (Diff revision 1) -
I agree with your observation that the first (and deleted) record has a better Comment string. But without looking at the code I do not want to go making changes to the wording--my goal was just to eliminate obvious duplicates.
-
indra/newview/app_settings/settings.xml (Diff revision 1) -
The viewer uses the last record of a repeated group, so it in this case it is using 10.0 (just to be sure I started the viewer and looked).
-
indra/newview/app_settings/settings.xml (Diff revision 1) -
If I had to make a guess I would say that the 2nd entry for Outbandwidth was the original and that someone copied it up 1 level and put in a comment "Expand render stats display" but forgot to change the associated key. One could argue that having a F32 is wrong and that it should be an unsigned integer, depending on what units are being used (i.e. a limit of 3.25 makes sense if you are using kb/sec).
Review request changed
Updated (Dec. 23, 2010, 12:12 p.m.)
- changed from Settings.xml: redundant entries and unnecessary tag to VWR-24100 Settings.xml: redundant entries and unnecessary tag
Other reviews