Bug 151491

Summary: Fix coding style of Intl code
Product: WebKit Reporter: Sukolsak Sakshuwong <sukolsak>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, darin, ggaren, keith_miller, mark.lam, msaboff, saam, sukolsak
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Sukolsak Sakshuwong
Reported 2015-11-20 05:18:31 PST
Fix coding style of Intl code according to Darin's comments in https://bugs.webkit.org/show_bug.cgi?id=147604 This patch does three things: 1. Rename pointers and references to ExecState from "exec" to "state". 2. Pass parameters by references instead of pointers if the parameters are required. 3. Remove the word "get" from the names of functions that don't return values through out arguments. This should make future Intl patches smaller.
Attachments
Patch (59.55 KB, patch)
2015-11-20 05:23 PST, Sukolsak Sakshuwong 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Sukolsak Sakshuwong
Comment 1 2015-11-20 05:23:22 PST
Created attachment 265950 [details] Patch
Mark Lam
Comment 2 2015-11-20 09:09:28 PST
Comment on attachment 265950 [details] Patch Hmmm, I know Darin said to use "state" instead of "exec" for the ExecState*, but I wonder if we should do that. Using "exec" to mean "ExecState*" is universally practiced in JSC code. Changing to use "state" in just some places would actually make it harder to read.
Darin Adler
Comment 3 2015-11-20 15:56:49 PST
(In reply to comment #2) > Hmmm, I know Darin said to use "state" instead of "exec" for the ExecState*, > but I wonder if we should do that. Using "exec" to mean "ExecState*" is > universally practiced in JSC code. Changing to use "state" in just some > places would actually make it harder to read. If you look you will see that we are starting to change "exec" to "state" in much new code, at least in WebCore.
Mark Lam
Comment 4 2015-11-20 16:20:37 PST
(In reply to comment #3) > (In reply to comment #2) > > Hmmm, I know Darin said to use "state" instead of "exec" for the ExecState*, > > but I wonder if we should do that. Using "exec" to mean "ExecState*" is > > universally practiced in JSC code. Changing to use "state" in just some > > places would actually make it harder to read. > > If you look you will see that we are starting to change "exec" to "state" in > much new code, at least in WebCore.
Mark Lam
Comment 5 2015-11-20 16:22:08 PST
Sigh ... such a rookie mistake (clicked Save before adding comment) ... (In reply to comment #3) > If you look you will see that we are starting to change "exec" to "state" in > much new code, at least in WebCore. OK. I'm good with this if this is the new direction we're taking.
Darin Adler
Comment 6 2015-11-20 18:30:18 PST
There might be people who don’t agree with me, but I think it’s a good change that we should eventually do everywhere.
Saam Barati
Comment 7 2015-11-20 19:48:56 PST
My vote would be to rename all ExecState to CallFrame. And we have a natural name for the variable: (CallFrame* callFrame)
WebKit Commit Bot
Comment 8 2015-11-30 14:21:31 PST
Comment on attachment 265950 [details] Patch Clearing flags on attachment: 265950 Committed r192831: <http://trac.webkit.org/changeset/192831>
WebKit Commit Bot
Comment 9 2015-11-30 14:21:35 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.