Bug 83431 – Cleanup wtf/Platform.h and config.h files

• Component: Web Template Framework
• Status: RESOLVED FIXED

[Patrick R. Gansterer]

Cleanup Platform.h

[Patrick R. Gansterer]

Created attachment 136145 [details]
Patch

[Early Warning System Bot]

Comment on attachment 136145 [details]
Patch

Attachment 136145 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12364730

[Early Warning System Bot]

Comment on attachment 136145 [details]
Patch

Attachment 136145 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/12364732

[Build Bot]

Comment on attachment 136145 [details]
Patch

Attachment 136145 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12367297

[Build Bot]

Comment on attachment 136145 [details]
Patch

Attachment 136145 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12364731

[Patrick R. Gansterer]

Created attachment 136150 [details]
Patch

[Alexey Proskuryakov]

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?

[Patrick R. Gansterer]

(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.

[Eric Seidel (no email)]

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!

[WebKit Review Bot]

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

[Patrick R. Gansterer]

Committed r113798: <http://trac.webkit.org/changeset/113798>