webkit-patch needs --verbose flag to enable DEBUG logging
Created attachment 56222 [details]
Comment on attachment 56222 [details]
Some thoughts to consider prior to landing. r=me
> + is_verbose = "-v" in sys.argv or "--verbose" in sys.argv
I wonder if there is a nice way to avoid writing sys.argv twice -- e.g.
Python set intersection?
> + def _command_for_printing(self, args):
> + """Used for producing a print-ready string representing command args.
> + The string should be copy/paste ready for execution in a shell."""
PEP8 says the doc string for functions should begin with an imperative
word, e.g. "Return a print-ready string...."
PEP8 also has pretty specific rules about the formatting of doc strings. This
one would look something like this--
def _command_for_printing(self, args):
"""Return a print-ready string that represents command args.
The string should be copy/paste ready for execution in a shell.
> + if isinstance(arg, unicode):
> + # Escape any non-ascii characters for easy copy/paste
> + arg = arg.encode("unicode_escape")
> + # quotes?
What does the "quotes?" comment mean -- is that a FIXME?
> + _log.debug("\"%s\" took %.2fs" % (self._command_for_printing(args), time.time() - start_time))
I would use single quotes to avoid escaping the double quotes.
Addressed Chris's comments. landing.
Committed r60066: <http://trac.webkit.org/changeset/60066>