Bug 36629 - Move commit_message_for_this_commit from scm to checkout
Summary: Move commit_message_for_this_commit from scm to checkout
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-03-25 17:12 PDT by Adam Barth
Modified: 2010-03-26 14:51 PDT (History)
2 users (show)

See Also:

Attachments
Patch (8.81 KB, patch)
2010-03-25 17:15 PDT, Adam Barth 		no flags Details | Formatted Diff | Diff
Patch (7.05 KB, patch)
2010-03-25 23:08 PDT, Eric Seidel (no email) 		no flags Details | Formatted Diff | Diff
Patch (7.34 KB, patch)
2010-03-26 00:15 PDT, Eric Seidel (no email) 		no flags Details | Formatted Diff | Diff
Patch for landing (1.74 KB, patch)
2010-03-26 13:25 PDT, Eric Seidel (no email) 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Barth 2010-03-25 17:12:04 PDT 
Move commit_message_for_this_commit from scm to checkout
Comment 1 Adam Barth 2010-03-25 17:15:08 PDT 
Created attachment 51695 [details]
Patch
Comment 2 Eric Seidel (no email) 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?
Comment 3 Adam Barth 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.
Comment 4 WebKit Commit Bot 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>
Comment 5 WebKit Commit Bot 2010-03-25 20:04:33 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 6 Eric Seidel (no email) 2010-03-25 21:49:00 PDT 
Committed r56599: <http://trac.webkit.org/changeset/56599>
Comment 7 Eric Seidel (no email) 2010-03-25 23:08:53 PDT 
Created attachment 51715 [details]
Patch
Comment 8 Eric Seidel (no email) 2010-03-25 23:09:15 PDT 
I should have opened a new bug, but oh well.
Comment 9 Adam Barth 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.
Comment 10 Eric Seidel (no email) 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.
Comment 11 Eric Seidel (no email) 2010-03-26 00:15:50 PDT 
Created attachment 51717 [details]
Patch
Comment 12 Eric Seidel (no email) 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.
Comment 13 Adam Barth 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.
Comment 14 Eric Seidel (no email) 2010-03-26 12:46:16 PDT 
Argued out in person.
Comment 15 Eric Seidel (no email) 2010-03-26 12:48:15 PDT 
Committed r56638: <http://trac.webkit.org/changeset/56638>
Comment 16 Eric Seidel (no email) 2010-03-26 13:25:53 PDT 
Created attachment 51775 [details]
Patch for landing
Comment 17 Eric Seidel (no email) 2010-03-26 13:26:21 PDT 
Re-open to let the commit-queue see this bug.
Comment 18 Eric Seidel (no email) 2010-03-26 14:14:17 PDT 
Comment on attachment 51775 [details]
Patch for landing

Hacking the commit-queue to land this first.
Comment 19 Eric Seidel (no email) 2010-03-26 14:48:18 PDT 
Comment on attachment 51775 [details]
Patch for landing

Make our tools work!
Comment 20 Eric Seidel (no email) 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>
Comment 21 Eric Seidel (no email) 2010-03-26 14:51:44 PDT 
All reviewed patches have been landed.  Closing bug.