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 treegraph
 
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

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.