Bug 151549 - Rename JavaScriptCore builtins files to match exposed object names
Summary: Rename JavaScriptCore builtins files to match exposed object names
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: BJ Burg
URL:
Keywords:
Depends on:
Blocks: 150482
  Show dependency treegraph
 
Reported: 2015-11-22 10:46 PST by BJ Burg
Modified: 2015-11-23 14:10 PST (History)
3 users (show)

See Also:


Attachments
Proposed Fix (112.43 KB, patch)
2015-11-22 11:05 PST, BJ Burg
no flags Details | Formatted Diff | Diff
Proposed Fix (18.54 KB, patch)
2015-11-22 11:08 PST, BJ Burg
youennf: review+
youennf: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description BJ Burg 2015-11-22 10:46:57 PST
As a subtask of unifying code generation for WebCore and JSC builtins, we need to get rid of differences between builtins filenames (e.g., Operations.Promise.js, Promise.prototype.js) and the name of the generated Builtin object (OperationsPromise, PromisePrototype). If we don't do this, then both build systems need special hacks to normalize the object name from the file name. It's easier to just normalize the filename.
Comment 1 BJ Burg 2015-11-22 11:05:59 PST
Created attachment 266057 [details]
Proposed Fix
Comment 2 BJ Burg 2015-11-22 11:08:40 PST
Created attachment 266058 [details]
Proposed Fix
Comment 3 youenn fablet 2015-11-22 11:30:15 PST
Comment on attachment 266058 [details]
Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=266058&action=review

Looks like the right direction to me.

> Source/JavaScriptCore/CMakeLists.txt:1198
>      ${JAVASCRIPTCORE_DIR}/builtins/GlobalObject.js

This filename has no suffix. Is it ok? Should we think of renaming it also?

> Source/JavaScriptCore/CMakeLists.txt:1204
> +    ${JAVASCRIPTCORE_DIR}/builtins/OperationsPromise.js

I would rename it to PromiseXXX.js so that it gets closer to other Promise related files.
If we were to follow WebCore, it would be named PromiseInternals.js, which would relate to @internal annotation.
Comment 4 BJ Burg 2015-11-22 16:53:52 PST
Comment on attachment 266058 [details]
Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=266058&action=review

>> Source/JavaScriptCore/CMakeLists.txt:1198
>>      ${JAVASCRIPTCORE_DIR}/builtins/GlobalObject.js
> 
> This filename has no suffix. Is it ok? Should we think of renaming it also?

I think it's okay, it's a singleton like ReflectObject and InspectorInstrumentationObject, and it can't be constructed.

>> Source/JavaScriptCore/CMakeLists.txt:1204
>> +    ${JAVASCRIPTCORE_DIR}/builtins/OperationsPromise.js
> 
> I would rename it to PromiseXXX.js so that it gets closer to other Promise related files.
> If we were to follow WebCore, it would be named PromiseInternals.js, which would relate to @internal annotation.

OK
Comment 5 youenn fablet 2015-11-22 23:02:25 PST
Comment on attachment 266058 [details]
Proposed Fix

r=me
Comment 6 BJ Burg 2015-11-23 14:10:16 PST
Committed r192751: <http://trac.webkit.org/changeset/192751>