Summary: | webkit-patch should warn users when they're using a 32-bit git on a 64-bit system | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Eric Seidel (no email) <eric> | ||||||||
Component: | New Bugs | Assignee: | Eric Seidel (no email) <eric> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | abarth, commit-queue, eric, kling, mihaip, ojan | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Other | ||||||||||
OS: | OS X 10.5 | ||||||||||
Attachments: |
|
Description
Eric Seidel (no email)
2010-12-08 14:03:39 PST
Created attachment 75960 [details]
Patch
Comment on attachment 75960 [details]
Patch
Hmm... Nevermind, platform.architecture() is not working as expected. Investigating.
Created attachment 75974 [details]
Patch
Created attachment 75977 [details]
move _check_git_architecture to after SCM.__init__
Comment on attachment 75977 [details] move _check_git_architecture to after SCM.__init__ View in context: https://bugs.webkit.org/attachment.cgi?id=75977&action=review > WebKitTools/ChangeLog:19 > + This message will print twice during 'webkit-patch upload', You could keep track of whether or not you printed out the message in a static, so that it only shows up once. (In reply to comment #5) > (From update of attachment 75977 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=75977&action=review > > > WebKitTools/ChangeLog:19 > > + This message will print twice during 'webkit-patch upload', > > You could keep track of whether or not you printed out the message in a static, so that it only shows up once. webkit-patch calls out to check-webkit-style (a separate process). Comment on attachment 75977 [details] move _check_git_architecture to after SCM.__init__ View in context: https://bugs.webkit.org/attachment.cgi?id=75977&action=review > WebKitTools/ChangeLog:23 > + the code is. I considered writing a test for the log message, but it seemed not worth it. I think testing _check_git_architecture is worthwhile just to ensure that the code actually runs. Don't even need to verify the output. It helps avoid simple programming errors (e.g. typos). (In reply to comment #7) > (From update of attachment 75977 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=75977&action=review > > > WebKitTools/ChangeLog:23 > > + the code is. I considered writing a test for the log message, but it seemed not worth it. > > I think testing _check_git_architecture is worthwhile just to ensure that the code actually runs. Don't even need to verify the output. It helps avoid simple programming errors (e.g. typos). Possible, however having git installed is not required for WebKit. So I could check that it throws an OSError or returns a value? The commit-queue encountered the following flaky tests while processing attachment 75977 [details]: fast/dom/onerror-img.html bug 51019 fast/preloader/script.html bug 50879 (author: abarth@webkit.org) The commit-queue is continuing to process your patch. The commit-queue encountered the following flaky tests while processing attachment 75977 [details]: http/tests/xmlhttprequest/cross-site-denied-response-sync.html bug 51020 (author: ap@webkit.org) The commit-queue is continuing to process your patch. Comment on attachment 75977 [details] move _check_git_architecture to after SCM.__init__ Clearing flags on attachment: 75977 Committed r74008: <http://trac.webkit.org/changeset/74008> All reviewed patches have been landed. Closing bug. |