From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Glass Date: Tue, 26 Mar 2013 15:12:38 -0700 Subject: [U-Boot] [PATCH 1/8] patman: Fix up checkpatch parsing to deal with 'CHECK' lines In-Reply-To: References: <1363833781-14557-1-git-send-email-sjg@chromium.org> <1363833781-14557-2-git-send-email-sjg@chromium.org> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Doug, On Thu, Mar 21, 2013 at 9:35 AM, Doug Anderson wrote: > Simon, > > On Wed, Mar 20, 2013 at 7:42 PM, Simon Glass wrote: >> checkpatch has a new type of warning, a 'CHECK'. At present patman fails >> with these, which makes it less than useful. >> >> Add support for checks, making it backwards compatible with the old >> checkpatch. > > There are also a few other minor fixups in this: > * Cleanup formatting of the CheckPatches() output. > * Fix erroneous "internal error" if multiple patches have warnings. > * Be more robust to new types of problems. Yes - I'll add them to the commit message. > > >> @@ -64,10 +64,14 @@ def CheckPatch(fname, verbose=False): >> 'msg': text message >> 'file' : filename >> 'line': line number >> + error_count: Number of errors >> + warning_count: Number of warnings >> + check_count: Number of checks >> lines: Number of lines >> + stdout: Full output of checkpatch > > nit: Right above this it says you're returning a 4-tuple. That's no > longer true. Could just remove the "4-". > > optional: I've found that returning big tuples like this is > problematic. Whenever you change the number of things returned you've > got to modify any callers that call like: > > a, b, c, d = CheckPatch(...) > > to now be: > > a, b, c, d, e = CheckPatch(...) > > ...or never use the above syntax and use: > > result = CheckPatch(...) > blah = result[0] > > Maybe use a namedtuple so that callers can use the result more cleanly? Yes, we are well past the point of needing something like that, so will add it. > > >> if match: >> error_count = int(match.group(1)) >> warning_count = int(match.group(2)) >> - lines = int(match.group(3)) >> + match_count = int(match.group(3)) >> + if len(match.groups()) == 4: >> + check_count = match_count >> + lines = int(match.group(4)) > > I'm confused about match_count here. What is it supposed to contain? > I can't tell from the name of it. It looks like a temporary variable > holding either check_count or lines. ...but you forget to assign > "lines = match_count" in an "else" case so things are broken with old > versions of checkpatch, right? Yes - we don't currently use the return value. But I will fix it. Regards, Simon > > > -Doug