Summary: | Clean up: Make regular expressions for parsing the start of an SVN and Git header global variables | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Daniel Bates <dbates> | ||||||||
Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | cjerdonek, eric | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
Daniel Bates
2010-05-09 01:12:19 PDT
Created attachment 55494 [details]
Patch
I did not add any additional unit tests as our existing test coverage should be sufficient.
Created attachment 55495 [details]
Patch
Based on a Chris's suggestion, stored the regular expressions in global variables instead of encapsulating them in functions.
Comment on attachment 55495 [details] Patch > +++ WebKitTools/ChangeLog (working copy) > + Extract the regular expressions for parsing the start of an SVN and > + Git header into their own respective functions so as to centralize > + these regular expressions as they are used throughout VCSUtils.pm. As we discussed, this needs to be updated. > +++ WebKitTools/Scripts/VCSUtils.pm (working copy) > +my $gitHeaderStartRegEx = qr#^diff --git (\w/)?(.+) (\w/)?([^\r\n]+)#; > +my $svnHeaderStartRegEx = qr#^Index: ([^\r\n]+)#; Might it be more straightforward to call these gitDiffStartRegEx and svnDiffStartRegEx? It's probably best to re-post after this since the latter involves a bunch of changes. (In reply to comment #3) > (From update of attachment 55495 [details]) > > +++ WebKitTools/ChangeLog (working copy) > > + Extract the regular expressions for parsing the start of an SVN and > > + Git header into their own respective functions so as to centralize > > + these regular expressions as they are used throughout VCSUtils.pm. > > As we discussed, this needs to be updated. Will change. > > > +++ WebKitTools/Scripts/VCSUtils.pm (working copy) > > +my $gitHeaderStartRegEx = qr#^diff --git (\w/)?(.+) (\w/)?([^\r\n]+)#; > > +my $svnHeaderStartRegEx = qr#^Index: ([^\r\n]+)#; > > Might it be more straightforward to call these gitDiffStartRegEx and svnDiffStartRegEx? Will change. > > It's probably best to re-post after this since the latter involves a bunch of changes. Will repost with changes. Created attachment 55498 [details]
Patch
Updated patch based on Chris's suggestions.
Comment on attachment 55498 [details]
Patch
Looks good. Thanks for this refactoring!
Comment on attachment 55498 [details] Patch Clearing flags on attachment: 55498 Committed r59051: <http://trac.webkit.org/changeset/59051> All reviewed patches have been landed. Closing bug. |