Bug 36629

Summary: Move commit_message_for_this_commit from scm to checkout
Product: WebKit Reporter: Adam Barth <abarth>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, eric
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch for landing none

Adam Barth
Reported 2010-03-25 17:12:04 PDT
Move commit_message_for_this_commit from scm to checkout
Attachments
Patch (8.81 KB, patch)
2010-03-25 17:15 PDT, Adam Barth 		no flags
DetailsFormatted DiffDiff
Patch (7.05 KB, patch)
2010-03-25 23:08 PDT, Eric Seidel (no email) 		no flags
DetailsFormatted DiffDiff
Patch (7.34 KB, patch)
2010-03-26 00:15 PDT, Eric Seidel (no email) 		no flags
DetailsFormatted DiffDiff
Patch for landing (1.74 KB, patch)
2010-03-26 13:25 PDT, Eric Seidel (no email) 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Adam Barth
Comment 1 2010-03-25 17:15:08 PDT
Created attachment 51695 [details] Patch
Eric Seidel (no email)
Comment 2 2010-03-25 17:16:44 PDT
Comment on attachment 51695 [details] Patch Do you mean to move the FIXME? Did you need to delete a mock after this?
Adam Barth
Comment 3 2010-03-25 17:28:48 PDT
Comment on attachment 51695 [details] Patch Yeah, I need to remove the FIXME. There's no mock to remove because our SCM mock inherits from Mock and is really loose.
WebKit Commit Bot
Comment 4 2010-03-25 20:04:28 PDT
Comment on attachment 51695 [details] Patch Clearing flags on attachment: 51695 Committed r56592: <http://trac.webkit.org/changeset/56592>
WebKit Commit Bot
Comment 5 2010-03-25 20:04:33 PDT
All reviewed patches have been landed. Closing bug.
Eric Seidel (no email)
Comment 6 2010-03-25 21:49:00 PDT
Committed r56599: <http://trac.webkit.org/changeset/56599>
Eric Seidel (no email)
Comment 7 2010-03-25 23:08:53 PDT
Created attachment 51715 [details] Patch
Eric Seidel (no email)
Comment 8 2010-03-25 23:09:15 PDT
I should have opened a new bug, but oh well.
Adam Barth
Comment 9 2010-03-25 23:11:48 PDT
Comment on attachment 51715 [details] Patch Doesn't this test leave junk files in the file system? We shouldn't be writing to the actual file system. We should mock out that part too.
Eric Seidel (no email)
Comment 10 2010-03-25 23:13:05 PDT
I had it different originally. It was using a temp svn checkout. Then I realized I didn't need the checkout. I'll fix.
Eric Seidel (no email)
Comment 11 2010-03-26 00:15:50 PDT
Created attachment 51717 [details] Patch
Eric Seidel (no email)
Comment 12 2010-03-26 00:18:15 PDT
We could split this method up into multiple smaller methods to allow easier mocking. But for now I've just used a temporary directory for holding these ChagneLogs. If we're adding more API tests which don't want the setUp/tearDown methods, then we should consider making this its own Test class, or using try/finally to make sure the setup/teardown is always run.
Adam Barth
Comment 13 2010-03-26 12:28:58 PDT
Comment on attachment 51717 [details] Patch I don't think we should be writing to the file system in a unit test.
Eric Seidel (no email)
Comment 14 2010-03-26 12:46:16 PDT
Argued out in person.
Eric Seidel (no email)
Comment 15 2010-03-26 12:48:15 PDT
Committed r56638: <http://trac.webkit.org/changeset/56638>
Eric Seidel (no email)
Comment 16 2010-03-26 13:25:53 PDT
Created attachment 51775 [details] Patch for landing
Eric Seidel (no email)
Comment 17 2010-03-26 13:26:21 PDT
Re-open to let the commit-queue see this bug.
Eric Seidel (no email)
Comment 18 2010-03-26 14:14:17 PDT
Comment on attachment 51775 [details] Patch for landing Hacking the commit-queue to land this first.
Eric Seidel (no email)
Comment 19 2010-03-26 14:48:18 PDT
Comment on attachment 51775 [details] Patch for landing Make our tools work!
Eric Seidel (no email)
Comment 20 2010-03-26 14:51:38 PDT
Comment on attachment 51775 [details] Patch for landing Clearing flags on attachment: 51775 Committed r56643: <http://trac.webkit.org/changeset/56643>
Eric Seidel (no email)
Comment 21 2010-03-26 14:51:44 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.