| Summary: | New port: EFL; adding files to WebCore/graphics/efl (patch 2 of 4) | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Leandro Pereira <leandro> | ||||||||
| Component: | Platform | Assignee: | Nobody <webkit-unassigned> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Enhancement | CC: | barbieri, commit-queue, eric, gustavo, gyuyoung.kim, kenneth, oliver, rakuco, tonikitoo, webkit.review.bot, zimmermann | ||||||||
| Priority: | P2 | ||||||||||
| Version: | 528+ (Nightly build) | ||||||||||
| Hardware: | Other | ||||||||||
| OS: | Linux | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Leandro Pereira
2010-03-01 13:01:38 PST
Created attachment 49743 [details]
Add files to WebCore/platform/graphics/efl (patch 2/4)
Comment on attachment 49743 [details]
Add files to WebCore/platform/graphics/efl (patch 2/4)
You don't split your patches very logically... This patch has Gstreamer code, but the previous one had as well, mixing it with IntRect, Image code etc.
You should really make one patch with all media related code etc. Try to split the patches logically.
(In reply to comment #2) > (From update of attachment 49743 [details]) > You should really make one patch with all media related code etc. Try to split > the patches logically. These patches were splitted by a script I wrote. This script only checks for the final patch size: it doesn't group the files per feature or similarity. The port is quite large to split everything by hand (not to say it is error prone), and making the script aware of related code would take time I don't have. Do a sort of the files and look for similar names? Shouldn't take much time to code. Landing takes time, so if you are interested in landing and maintaining a new port, which your team has voiced on the mailing list, you should definitely have the time for landing the initial patches correctly. The reviewers are doing a service for you, so it is your job making it as easy as possible for them. Created attachment 49825 [details]
Add EFL port files in WebCore/platform/graphics/efl (patch 2/2)
Comment on attachment 49825 [details]
Add EFL port files in WebCore/platform/graphics/efl (patch 2/2)
I don't know enough to say if the GlyphPage part is correct, but the rest looks good to me. I guess that GlyphPage code is similar to the GTK+ one right?
(In reply to comment #6) > (From update of attachment 49825 [details]) > I don't know enough to say if the GlyphPage part is correct, but the rest looks > good to me. I guess that GlyphPage code is similar to the GTK+ one right? They're identical, actually. Moving it to platform/graphics/cairo would make sense. Comment on attachment 49825 [details]
Add EFL port files in WebCore/platform/graphics/efl (patch 2/2)
r- for above reason.
(In reply to comment #8) > (From update of attachment 49825 [details]) > r- for above reason. I've submitted bug #35695, which moves these files to a neutral cairo directory. Created attachment 50009 [details]
Add EFL port files to WebCore/platform/graphics/efl (2/2)
Comment on attachment 50009 [details] Add EFL port files to WebCore/platform/graphics/efl (2/2) Clearing flags on attachment: 50009 Committed r55526: <http://trac.webkit.org/changeset/55526> All reviewed patches have been landed. Closing bug. |