Bug 100548 - [CSS Shaders] Extract the CustomFilterParameterList to its own file
Summary: [CSS Shaders] Extract the CustomFilterParameterList to its own file
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alexandru Chiculita
URL:
Keywords:
Depends on:
Blocks: 100533
  Show dependency treegraph
 
Reported: 2012-10-26 11:52 PDT by Alexandru Chiculita
Modified: 2012-10-29 09:50 PDT (History)
8 users (show)

See Also:


Attachments
Patch V1 (22.12 KB, patch)
2012-10-26 13:34 PDT, Alexandru Chiculita
webkit-ews: commit-queue-
Details | Formatted Diff | Diff
Patch V2 (22.97 KB, patch)
2012-10-26 15:03 PDT, Alexandru Chiculita
webkit-ews: commit-queue-
Details | Formatted Diff | Diff
Patch V3 (23.06 KB, patch)
2012-10-26 15:20 PDT, Alexandru Chiculita
dino: review+
eflews.bot: commit-queue-
Details | Formatted Diff | Diff
Patch for landing (23.07 KB, patch)
2012-10-29 09:22 PDT, Alexandru Chiculita
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexandru Chiculita 2012-10-26 11:52:44 PDT
CustomFilterParameterList is defined as a typedef in the header of CustomFilterOperation. We will reuse the CustomFilterParameterList in different other places, so extract that to its own file.
Comment 1 Alexandru Chiculita 2012-10-26 13:34:00 PDT
Created attachment 170993 [details]
Patch V1
Comment 2 Early Warning System Bot 2012-10-26 13:42:33 PDT
Comment on attachment 170993 [details]
Patch V1

Attachment 170993 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/14610169
Comment 3 Alexandru Chiculita 2012-10-26 15:03:58 PDT
Created attachment 171025 [details]
Patch V2
Comment 4 WebKit Review Bot 2012-10-26 15:09:36 PDT
Attachment 171025 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/CMakeLists.txt', u'Source/W..." exit_code: 1
Source/WebCore/platform/graphics/filters/CustomFilterParameterList.h:43:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/platform/graphics/filters/CustomFilterParameterList.h:44:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 2 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Early Warning System Bot 2012-10-26 15:12:32 PDT
Comment on attachment 171025 [details]
Patch V2

Attachment 171025 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/14593824
Comment 6 Early Warning System Bot 2012-10-26 15:17:19 PDT
Comment on attachment 171025 [details]
Patch V2

Attachment 171025 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/14617145
Comment 7 Alexandru Chiculita 2012-10-26 15:20:57 PDT
Created attachment 171026 [details]
Patch V3
Comment 8 Max Vujovic 2012-10-26 16:58:28 PDT
Comment on attachment 171026 [details]
Patch V3

Looks good, Alex.

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

> Source/WebCore/platform/graphics/filters/CustomFilterParameterList.cpp:49
> +bool CustomFilterParameterList::operator==(const CustomFilterParameterList& other) const

Nice. An operator== looks much better than customFilterParametersEqual(...).

> Source/WebCore/platform/graphics/filters/CustomFilterParameterList.h:45
> +    explicit CustomFilterParameterList(size_t);

Nice use of explicit.
Comment 9 EFL EWS Bot 2012-10-26 16:59:43 PDT
Comment on attachment 171026 [details]
Patch V3

Attachment 171026 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/14608279
Comment 10 Dean Jackson 2012-10-29 07:07:02 PDT
Comment on attachment 171026 [details]
Patch V3

I like this patch. Not sure why EFL couldn't find the new file, but assuming that works itself out, I'm happy.
Comment 11 Raphael Kubo da Costa (:rakuco) 2012-10-29 07:14:03 PDT
Comment on attachment 171026 [details]
Patch V3

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

> Source/WebCore/CMakeLists.txt:1914
> +    platform/graphics/filters/CustomFilterParameter.cpp

The file is called CustomFilterParameterList.cpp, not CustomFilterParameter.cpp.
Comment 12 Dean Jackson 2012-10-29 08:13:29 PDT
(In reply to comment #11)
> (From update of attachment 171026 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=171026&action=review
> 
> > Source/WebCore/CMakeLists.txt:1914
> > +    platform/graphics/filters/CustomFilterParameter.cpp
> 
> The file is called CustomFilterParameterList.cpp, not CustomFilterParameter.cpp.

Ah, thanks! No need to r- then because it is an easy fix.
Comment 13 Alexandru Chiculita 2012-10-29 08:42:30 PDT
(In reply to comment #12)
> (In reply to comment #11)
> > (From update of attachment 171026 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=171026&action=review
> > 
> > > Source/WebCore/CMakeLists.txt:1914
> > > +    platform/graphics/filters/CustomFilterParameter.cpp
> > 
> > The file is called CustomFilterParameterList.cpp, not CustomFilterParameter.cpp.
> 
> Ah, thanks! No need to r- then because it is an easy fix.

Thanks, Dean! Sorry about the naming, it's the evil copy-paste :)
Comment 14 Alexandru Chiculita 2012-10-29 09:22:37 PDT
Created attachment 171258 [details]
Patch for landing
Comment 15 WebKit Review Bot 2012-10-29 09:50:25 PDT
Comment on attachment 171258 [details]
Patch for landing

Clearing flags on attachment: 171258

Committed r132808: <http://trac.webkit.org/changeset/132808>
Comment 16 WebKit Review Bot 2012-10-29 09:50:30 PDT
All reviewed patches have been landed.  Closing bug.