[MERGE] Support 3 noise levels in commit

John Arbash Meinel john at arbash-meinel.com
Thu Aug 23 18:46:46 BST 2007


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Aaron Bentley wrote:
> Ian Clatworthy wrote:
>> This was discussed some time back here:
>> https://lists.ubuntu.com/archives/bazaar/2007q2/027044.html. As that
>> thread indicates, everyone who voiced an opinion agreed at the time.
>> Let's hope everyone still does. :-) (If not, you can always add an alias
>> for commit that switches --verbose back on by default.)
> 
>> Running some profiling today, I noticed that 15% of an initial commit on
>> a Python tree (3726 files) was generating output even when output was
>> redirected to /dev/null. The attached patch implements the multiple
>> noise levels improvement and delivers better performance by using that
>> level information to skip generating info if we know we're never going
>> to display it.
> 
>> Ian C.
> 
> I don't think this is the right way to achieve it.
> 
> a) --quiet already has a meaning in the codebase.  I don't think it
> makes sense to reuse it.  Also, does it work with aliases?

- --quiet (in the global sense) doesn't. Because of when aliases are
parsed versus when global arguments are parsed, you can't add a global
arg to aliases.

That said, I think '--quiet' should have an effect on commit. I don't
think he is mis-using it to have that sort of effect.

> 
> b) "bzr commit --quiet --verbose" should select "verbose", the way the
> rest of our options behave.
> 
> Instead, I think it makes more sense to treat verbose as an enumeration,
> with --silent, --normal and --verbose as individual flags.
> "verbose=False" can be handled as a legacy value.
> 

Again, global options muddy the waters here. We need to change how we
parse those before we can have that sort of effect.

>> @@ -728,7 +758,8 @@
>>          if ie is not None:
>>              self.builder.record_entry_contents(ie, self.parent_invs, 
>>                  path, self.work_tree)
>> -            self._report_change(ie, path)
>> +            if self.reporter.is_verbose():
>> +                self._report_change(ie, path)
> 
> ^^^ is this really a win?  You don't have newer function calls.  You may
> have *more*, in fact.
> 
> Aaron

I think it would be a net win if you cached the result in a local
variable, but otherwise I agree that you end up with 2 calls instead of 1.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGzcgGJdeBCYSNAAMRAiwEAJ4h5lAXWVkiDNrQ/XDi87WQ+LqKDQCgloDf
PuvhqKDhAKJRU7nmqX4gDP4=
=ulsC
-----END PGP SIGNATURE-----



More information about the bazaar mailing list