Summary: | svn-unapply doesn't revert directories correctly | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Cameron McCormack (:heycam) <heycam> | ||||||
Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Minor | CC: | commit-queue | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Mac (Intel) | ||||||||
OS: | OS X 10.5 | ||||||||
Attachments: |
|
Description
Cameron McCormack (:heycam)
2009-09-08 18:45:47 PDT
Created attachment 39242 [details]
Patch v1
The problem is that svnStatus() returns a status line with a newline from one branch, but without a newline from the other branch. That caused the regexes in revertDirectories() to fail to match, because they were looking for a \n at the end.
Comment on attachment 39242 [details]
Patch v1
it seems this repeated regexp really should be in a function, or at least as a constant.
Also, I think perl is slightly more readable when you make use of $_ explicit, so I prefer the old code.
Comment on attachment 39242 [details]
Patch v1
r- for my personal taste nits.
Created attachment 39309 [details] Patch v2 (In reply to comment #2) > it seems this repeated regexp really should be in a function, or at least as a > constant. Done. Note also I fixed a small bug -- the second s/// I added was being applied to $_ when it should have been applied to $svnStatus. > Also, I think perl is slightly more readable when you make use of $_ explicit, > so I prefer the old code. I'd argue that leaving off the "$_ =~" is more idiomatic Perl, but OK, done. Perhaps if I'd included it I would've picked up the above bug first time around. :-) Comment on attachment 39309 [details]
Patch v2
OK. I guess these could move to VCSTools.pm at some point...
I think it's about time to get you commit bit. I'll look into it.
Comment on attachment 39309 [details]
Patch v2
Rejecting patch 39309 from commit-queue.
This patch will require manual commit. ['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--quiet', '--exit-after-n-failures=1'] failed with exit code 1
Comment on attachment 39309 [details] Patch v2 The media madness never ends! compositing/color-matching/image-color-matching.html -> timed out bug 28624. Comment on attachment 39309 [details] Patch v2 Clearing flags on attachment: 39309 Committed r48242: <http://trac.webkit.org/changeset/48242> All reviewed patches have been landed. Closing bug. |