On Fri, Apr 2, 2010 at 4:19 PM, Stefan Reinauer <[email protected]>wrote:
> On 4/2/10 7:55 PM, Myles Watson wrote: > > loglevel.diff: > Remove BIOS_EMERG, BIOS_ALERT, BIOS_CRIT, BIOS_NOTICE, BIOS_NEVER > > I am not convinced that's a good idea. But let's hear what other people > say... > They were used so infrequently that they were just noise to me. > > I didn't change their numerical values. They were rarely used. > BIOS_EMERG, BIOS_ALERT, BIOS_CRIT -> BIOS_ERR > BIOS_NOTICE -> BIOS_INFO > > I don't think we want to just remove log levels like that. Let's rather fix > the messages and choose the right log levels for them. > I think it preserves the meaning. Emergency, alert, and critical all seem like errors. The difference between notice and info seems like splitting hairs. > > BIOS_NEVER -> BIOS_SPEW+1 > > The whole intent was to make clear to the reader that this is something > that's never seen. ;-) > That doesn't become clear from BIOS_SPEW+1 in my opinion. > NEVER was only used once or twice in the code. If it's something that will be used more often, then I can see your point. Why don't we just comment out messages that will never be seen? > > loglevel2.diff: > BIOS_ERR -> ERROR > BIOS_WARNING -> WARNING > BIOS_INFO -> INFO > BIOS_DEBUG -> DEBUG > BIOS_SPEW -> SPEW > For 80 character line aficionados, this puts most of the lines back, since > printk_debug(...) is about the same length as printk(DEBUG, ...) > > I kind of like the fact that they are prefixed, while I see the point that > BIOS_ is a bad prefix. What about LOG_? Because, that's what it is, log > levels. > I think printk works well for the prefix, and I like brevity when it doesn't interfere with the meaning. I think we talked about other prefixes some time in the past and couldn't get agreement. > I like the idea of not abbreviating ERROR to ERR as the only exception to > the rule. > Good. I've mis-typed that a lot of times. I like uniformity. Thanks, Myles
-- coreboot mailing list: [email protected] http://www.coreboot.org/mailman/listinfo/coreboot
