> 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. > > Gabe Black wrote: > 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. > > Steve Reinhardt wrote: > So you're suggesting converting them to real options, like --verbose? > That makes a lot of sense to me. And while we're on a consistency kick, it > should probably be --update-ref rather than --update_ref.
Ok, great. I'll make these changes and make it --update-ref instead of --update_ref. I'll update the diff once I get a chance. - 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
