Bug 53540

Summary: Rename PLATFORM(CF) to USE(CF)
Product: WebKit Reporter: Patrick R. Gansterer <paroga>
Component: New BugsAssignee: Patrick R. Gansterer <paroga>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, buildbot, commit-queue, darin, ddkilzer, dglazkov, gustavo, mjs, psolanki, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 58728    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch for landing none

Description Patrick R. Gansterer 2011-02-01 15:41:26 PST
Rename PLATFORM(CF) to USE(COREFOUNDATION)
Comment 1 Patrick R. Gansterer 2011-02-01 15:43:19 PST
Created attachment 80842 [details]
Patch
Comment 2 WebKit Review Bot 2011-02-01 15:59:02 PST
Attachment 80842 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7684754
Comment 3 Darin Adler 2011-02-01 17:20:01 PST
Comment on attachment 80842 [details]
Patch

Changing from PLATFORM to USE might be OK. But changing from CF to COREFOUNDATION is bad since we also use CF as suffixes on file names. This generally will make the source tree harder to read. If those file names did not exist, then I could see the argument since CF is quite a short acronym and many might not be aware of its meaning.
Comment 4 Darin Adler 2011-02-01 17:20:26 PST
Maciej, do you have an opinion on this?
Comment 5 Build Bot 2011-02-01 17:39:49 PST
Attachment 80842 [details] did not build on win:
Build output: http://queues.webkit.org/results/7687360
Comment 6 Gustavo Noronha (kov) 2011-02-01 18:03:41 PST
Attachment 80842 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/7681924
Comment 7 WebKit Review Bot 2011-02-01 21:58:46 PST
Attachment 80842 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7682899
Comment 8 WebKit Review Bot 2011-02-01 23:21:24 PST
Attachment 80842 [details] did not build on mac:
Build output: http://queues.webkit.org/results/7686729
Comment 9 Patrick R. Gansterer 2011-02-01 23:26:52 PST
Created attachment 80895 [details]
Patch

(In reply to comment #3)
> Changing from PLATFORM to USE might be OK. But changing from CF to COREFOUNDATION is bad since we also use CF as suffixes on file names. This generally will make the source tree harder to read. If those file names did not exist, then I could see the argument since CF is quite a short acronym and many might not be aware of its meaning.
I agree, the filename suffix is a good argument against renaming to COREFOUNDATION (at least in this patch)
Comment 10 Patrick R. Gansterer 2011-02-01 23:28:25 PST
Created attachment 80896 [details]
Patch
Comment 11 Patrick R. Gansterer 2011-02-01 23:32:07 PST
Created attachment 80897 [details]
Patch
Comment 12 Patrick R. Gansterer 2011-02-08 10:37:43 PST
@darin: ping
Comment 13 Patrick R. Gansterer 2011-02-22 12:58:35 PST
ping?
Comment 14 WebKit Commit Bot 2011-02-22 13:44:59 PST
Comment on attachment 80897 [details]
Patch

Rejecting attachment 80897 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-7', 'apply-..." exit_code: 2

Last 500 characters of output:
t 1 with fuzz 3.
patching file Source/WebKit2/UIProcess/WebBackForwardList.h
patching file Source/WebKit2/UIProcess/WebPageProxy.cpp
Hunk #1 succeeded at 961 (offset 115 lines).
patching file Source/WebKit2/WebProcess/WebCoreSupport/WebPlatformStrategies.cpp
patching file Tools/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Tools/DumpRenderTree/config.h

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

Full output: http://queues.webkit.org/results/7948289
Comment 15 Alexey Proskuryakov 2011-02-22 13:52:12 PST
Could someone please explain why USE(CF) is more appropriate than PLATFORM(CF)? I'm not saying that this change is wrong, but how can it be landed without any explanation?
Comment 16 Patrick R. Gansterer 2011-02-22 14:00:46 PST
(In reply to comment #15)
> Could someone please explain why USE(CF) is more appropriate than PLATFORM(CF)? I'm not saying that this change is wrong, but how can it be landed without any explanation?

CF is not a platform. CF is a library we can use. There are some old webkit-dev mails about "better names": http://thread.gmane.org/gmane.os.opendarwin.webkit.devel/4998/focus=5002.

IMHO the title of the bug is self explaining and I don't se much value in additional info in the ChangeLog. If you a good content for it I'll add it. ;-)
Comment 17 Alexey Proskuryakov 2011-02-22 14:54:49 PST
This doesn't have to be in ChangeLog.

What makes library a platform? WX and Qt are also technically libraries, so should we rename PLATFORM(QT) to USE(QT)? I honestly don't understand how the decision was made.
Comment 18 Patrick R. Gansterer 2011-02-22 22:16:23 PST
(In reply to comment #17)
> This doesn't have to be in ChangeLog.
> 
> What makes library a platform? WX and Qt are also technically libraries, so should we rename PLATFORM(QT) to USE(QT)? I honestly don't understand how the decision was made.

PLATFORM should be something be something representing a set of used libraries. E.g. Qt uses always Qt unicode, Qt graphichs, Qt network and so on. The CF stuff is used by Apples Mac and Windows port and some parts are used by WinCairo too.

My vision is to remove all PLATFORM macros and replace them with USE macros: PLATFORM(QT) will then renamed to USE(QT_UNICODE), USE(QT_GRAPHICS), USE(QT_NETWORK), USE(QT_XML) and so on. This will enable us to use Qt rendering with curl network backend and ICU unicode. This maybe doesn't make much sense, but on OS(WINDOWS) you can then choose between CG, Cairo and GDI for graphics rendering  and CF, CUrl, Soup and WinInet for networking.
Comment 19 Patrick R. Gansterer 2011-02-22 22:44:31 PST
Created attachment 83448 [details]
Patch for landing
Comment 20 WebKit Commit Bot 2011-02-23 03:31:34 PST
Comment on attachment 83448 [details]
Patch for landing

Clearing flags on attachment: 83448

Committed r79434: <http://trac.webkit.org/changeset/79434>
Comment 21 WebKit Commit Bot 2011-02-23 03:31:42 PST
All reviewed patches have been landed.  Closing bug.