> On 2011-03-02 09:21:38, Steve Reinhardt wrote: > > I'm willing to be convinced on this one, but I'd argue that the > > inconsistency is that VERBOSE should be lowercase, not that update_ref > > should be uppercase... I think I originally made update_ref lowercase to > > distinguish it from the sticky args that control the build itself, where > > the non-sticky vars are more like --foo command-line options (conceptually > > if not syntactically). > > > > Just some historical perspective from the guy who introduced update_ref.
Since I originally made this patch I ran into the "default" scons variable which sets what file in build_opts to draw from if you're using some new name that's not already in there. This is inconsistent too, but then it seemed like making it capital might be confusing since it's selecting what to use to set variables -except- itself and potentially others. It's not a huge inconsistency, but when I was thinking about it it felt like something somebody could trip over. I also thought about the syntactical inconsistency between VERBOSE and --color and --no-color. Both are changing something about how the build output looks, both are transient, but one is a scons argument and the other is a command line option. I remembered somebody saying scons already had a --verbose command line option, but looking for it just now I didn't see it. So how about this. All non-sticky options should be in the form of command line options in lower case. That would be default, color, no-color, update_ref, and verbose as far as I can tell, but default was hiding from me for a while so there may be some other one lurking in there. Then all the sticky options will be CAPITALIZED and left as scons arguments. That seems to be consistent as far as capitalization, and input mechanism relative to stickiness. - Gabe ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/522/#review922 ----------------------------------------------------------- On 2011-02-28 04:54:41, Gabe Black wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/522/ > ----------------------------------------------------------- > > (Updated 2011-02-28 04:54:41) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > ------- > > SCons: Clean up some inconsistent capitalization in scons options. > > > Diffs > ----- > > SConstruct c6ba129c2764 > src/mem/protocol/SConsopts c6ba129c2764 > tests/SConscript c6ba129c2764 > > Diff: http://reviews.m5sim.org/r/522/diff > > > Testing > ------- > > > Thanks, > > Gabe > > _______________________________________________ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
