[webkitpy] Refactor diff_parser.py

[webkitpy] Refactor diff_parser.py

Precompile all regexes to eliminate self developed regex caching.

BUG=none

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=183448

[elecro, 2014-10-07 17:22:26]

pgal.u-szeged@partner.samsung.com changed reviewers:
+ dpranke@chromium.org

[elecro, 2014-10-07 18:07:43]

PTAL

[Dirk Pranke, 2014-10-08 01:45:47]

https://codereview.chromium.org/637503004/diff/1/Tools/Scripts/webkitpy/commo...
File Tools/Scripts/webkitpy/common/checkout/diff_parser.py (right):

https://codereview.chromium.org/637503004/diff/1/Tools/Scripts/webkitpy/commo...
Tools/Scripts/webkitpy/common/checkout/diff_parser.py:90: diff_git_pattern =
re.compile(r"^diff --git \w/")
I'm not generally a fan of class-level constants that then get accessed by class
methods. Such things tend to make most sense when there might be subclasses that
will override these values, but that won't happen here.

We shouldn't use class-level fields as a code structuring mechanism; that's what
packages and modules are for. 

Can we move these to just be module-level constants and methods?

[elecro, 2014-10-08 08:14:16]

https://codereview.chromium.org/637503004/diff/1/Tools/Scripts/webkitpy/commo...
File Tools/Scripts/webkitpy/common/checkout/diff_parser.py (right):

https://codereview.chromium.org/637503004/diff/1/Tools/Scripts/webkitpy/commo...
Tools/Scripts/webkitpy/common/checkout/diff_parser.py:90: diff_git_pattern =
re.compile(r"^diff --git \w/")
On 2014/10/08 01:45:47, Dirk Pranke wrote:
> I'm not generally a fan of class-level constants that then get accessed by
class
> methods. Such things tend to make most sense when there might be subclasses
that
> will override these values, but that won't happen here.
> 
> We shouldn't use class-level fields as a code structuring mechanism; that's
what
> packages and modules are for. 
> 
> Can we move these to just be module-level constants and methods?

Yeah, we can move them, but the FIXMEs mentioned to move the methods into the
DiffParser :). So I'll update the patch and remove the fixmes also.

[elecro, 2014-10-08 10:19:47]

updated

[Dirk Pranke, 2014-10-08 14:17:51]

Thanks!

I suspect the fixmes were from a time when we thought differently about static
methods and class methods than we do now.

lgtm

[elecro, 2014-10-09 07:55:15]

The CQ bit was checked by pgal.u-szeged@partner.samsung.com

[commit-bot: I haz the power, 2014-10-09 07:56:16]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/637503004/20001

[commit-bot: I haz the power, 2014-10-09 10:18:10]

Committed patchset #2 (id:20001) as 183448