Bug 66294 - [WebSocket] Remove arguments from CloseEvent::create()
Summary: [WebSocket] Remove arguments from CloseEvent::create()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Trivial
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 66362
  Show dependency tree / graph
 
Reported: 2011-08-16 05:36 PDT by Takashi Toyoshima
Modified: 2011-08-18 01:03 PDT (History)
5 users (show)

See Also:

Attachments
Patch (2.48 KB, patch)
2011-08-16 05:42 PDT, Takashi Toyoshima 		no flags Details | Formatted Diff | Diff
Patch (2.71 KB, patch)
2011-08-17 02:27 PDT, Takashi Toyoshima 		no flags Details | Formatted Diff | Diff
Patch (2.66 KB, patch)
2011-08-17 02:50 PDT, Takashi Toyoshima 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Takashi Toyoshima 2011-08-16 05:36:35 PDT 
I took over implementation of CloseEvent's code and reason.
For the coming next change, I'd like to remove arguments from CloseEvent::create() because it was useless.
Comment 1 Takashi Toyoshima 2011-08-16 05:42:21 PDT 
Created attachment 104030 [details]
Patch
Comment 2 Yuta Kitamura 2011-08-16 05:56:49 PDT 
Comment on attachment 104030 [details]
Patch

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

[Note: I'm not a WebKit reviewer]

Code change looks fine to me.

> Source/WebCore/ChangeLog:7
> +

You need to write the reason why you can safely omit wasClean argument from create().

You also need to justify not adding a new test.
Comment 3 Takashi Toyoshima 2011-08-17 02:27:52 PDT 
Created attachment 104159 [details]
Patch
Comment 4 Yuta Kitamura 2011-08-17 02:42:30 PDT 
Comment on attachment 104159 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        Remove arguments from CloseEvent::create(). CloseEvent objects are
> +        initialized by initCloseEvent() in all cases. Initialization in
> +        create() is redundant. We does not need additional tests because this
> +        change does not introduce any functional difference.
> +        https://bugs.webkit.org/show_bug.cgi?id=66294
> +
> +        Reviewed by NOBODY (OOPS!).

Normally, ChangeLog entry is formatted like:

  <Bug title>
  <Bug URL>
  
  Reviewed by <Reviewer>.
  
  <Change description>

See existing entries.
Comment 5 Takashi Toyoshima 2011-08-17 02:50:45 PDT 
Created attachment 104163 [details]
Patch
Comment 6 Takashi Toyoshima 2011-08-17 02:52:15 PDT 
Sorry for bothering.
Revised again.
Comment 7 Yuta Kitamura 2011-08-17 04:42:32 PDT 
Comment on attachment 104163 [details]
Patch

Looks fine. [Note: I'm not a WebKit reviewer]
Comment 8 Kent Tamura 2011-08-17 23:44:21 PDT 
Comment on attachment 104163 [details]
Patch

ok
Comment 9 WebKit Review Bot 2011-08-18 01:02:55 PDT 
Comment on attachment 104163 [details]
Patch

Clearing flags on attachment: 104163

Committed r93288: <http://trac.webkit.org/changeset/93288>
Comment 10 WebKit Review Bot 2011-08-18 01:03:00 PDT 
All reviewed patches have been landed.  Closing bug.