Bug 83431 - Cleanup wtf/Platform.h and config.h files
Summary: Cleanup wtf/Platform.h and config.h files
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Patrick R. Gansterer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-04-08 04:07 PDT by Patrick R. Gansterer
Modified: 2012-04-10 18:09 PDT (History)
3 users (show)

See Also:


Attachments
Patch (8.25 KB, patch)
2012-04-08 04:35 PDT, Patrick R. Gansterer
no flags Details | Formatted Diff | Diff
Patch (12.28 KB, patch)
2012-04-08 11:16 PDT, Patrick R. Gansterer
eric: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick R. Gansterer 2012-04-08 04:07:08 PDT
Cleanup Platform.h
Comment 1 Patrick R. Gansterer 2012-04-08 04:35:04 PDT
Created attachment 136145 [details]
Patch
Comment 2 Early Warning System Bot 2012-04-08 04:50:48 PDT
Comment on attachment 136145 [details]
Patch

Attachment 136145 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12364730
Comment 3 Early Warning System Bot 2012-04-08 04:51:57 PDT
Comment on attachment 136145 [details]
Patch

Attachment 136145 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/12364732
Comment 4 Build Bot 2012-04-08 04:55:49 PDT
Comment on attachment 136145 [details]
Patch

Attachment 136145 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12367297
Comment 5 Build Bot 2012-04-08 05:29:14 PDT
Comment on attachment 136145 [details]
Patch

Attachment 136145 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12364731
Comment 6 Patrick R. Gansterer 2012-04-08 11:16:23 PDT
Created attachment 136150 [details]
Patch
Comment 7 Alexey Proskuryakov 2012-04-09 10:57:24 PDT
I tried svn blame on a random line where something was defined to 0, and that was added in a patch entitled "Clean up Platform.h" (http://trac.webkit.org/changeset/36074).

It's not cool to have cleanup patches move us in opposite directions. Can you find out the motivation behind that change, and also behind other changes that added the lines you're removing?
Comment 8 Patrick R. Gansterer 2012-04-09 11:09:46 PDT
(In reply to comment #7)
> I tried svn blame on a random line where something was defined to 0, and that was added in a patch entitled "Clean up Platform.h" (http://trac.webkit.org/changeset/36074).
> 
> It's not cool to have cleanup patches move us in opposite directions. Can you find out the motivation behind that change, and also behind other changes that added the lines you're removing?

I blamed a bunch of the "0 defines" too, but all of them seamed to me like "wrong code". E.g. in r36074 is no code change which depends on the 0. It only looks like a "quick" change with no accurate review to me.
I can contact all people who introduced "0 defines", but I don't think that will make much sense.
Comment 9 Eric Seidel (no email) 2012-04-10 11:05:51 PDT
Comment on attachment 136150 [details]
Patch

Thanks!  I think there have been different visions at different times about what belongs in config.h vs. platform.  I also think that some folks might argue it's nice to have all the #defin foo 0' around so we know what we can set to 1.  But I like your cleanup.  I like simpliflying these files, and I'm glad someone is finally looking at it!
Comment 10 WebKit Review Bot 2012-04-10 11:12:30 PDT
Comment on attachment 136150 [details]
Patch

Rejecting attachment 136150 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
 out of 1 hunk FAILED -- saving rejects to file Source/JavaScriptCore/config.h.rej
patching file Source/WTF/config.h
Hunk #1 FAILED at 54.
Hunk #2 succeeded at 82 (offset -3 lines).
1 out of 2 hunks FAILED -- saving rejects to file Source/WTF/config.h.rej
patching file Source/WTF/wtf/Platform.h
patching file Source/WebCore/config.h

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Eric Seidel']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue/

Full output: http://queues.webkit.org/results/12384313
Comment 11 Patrick R. Gansterer 2012-04-10 18:09:42 PDT
Committed r113798: <http://trac.webkit.org/changeset/113798>