[Webkit-unassigned] [Bug 39208] webkit-patch needs --verbose flag to enable DEBUG logging

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=39208


Chris Jerdonek <cjerdonek at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #56222|review?, commit-queue?      |review+, commit-queue-
               Flag|                            |




--- Comment #2 from Chris Jerdonek <cjerdonek at webkit.org>  2010-05-17 02:09:02 PST ---
(From update of attachment 56222)
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.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.