| Summary: | [GStreamer] Convert GStreamerUtilities in GStreamerCommon and include the GStreamer smart pointer traits | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Xabier Rodríguez Calvar <calvaris> | ||||||
| Component: | New Bugs | Assignee: | Xabier Rodríguez Calvar <calvaris> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | calvaris, cgarcia, commit-queue, eric.carlson, mcatanzaro, pnormand, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Xabier Rodríguez Calvar
2018-04-12 05:12:43 PDT
Created attachment 337789 [details]
Patch
Comment on attachment 337789 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=337789&action=review > Source/WebCore/ChangeLog:13 > + Why the rename? I'm not sure I get the new name Common, common to what? > Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp:24 > +#include "GStreamerCommon.h" The header is already guarded, so it can be moved right after config.h as suggested by the style guidelines. (In reply to Carlos Garcia Campos from comment #2) > Why the rename? I'm not sure I get the new name Common, common to what? Because I want to include the smart pointer headers there and that way is no longer a utility file, but something highly recommendable. > > Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp:24 > > +#include "GStreamerCommon.h" > > The header is already guarded, so it can be moved right after config.h as > suggested by the style guidelines. The style checker didn't mention it :/ Could I fix this during landing if you find the rest ok? (In reply to Xabier Rodríguez Calvar from comment #3) > (In reply to Carlos Garcia Campos from comment #2) > > Why the rename? I'm not sure I get the new name Common, common to what? > > Because I want to include the smart pointer headers there and that way is no > longer a utility file, but something highly recommendable. And smart pointers are not utilities? I find the rename confusing and unnecessary, but I won't oppose if it makes sense for others. > > > Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp:24 > > > +#include "GStreamerCommon.h" > > > > The header is already guarded, so it can be moved right after config.h as > > suggested by the style guidelines. > > The style checker didn't mention it :/ Could I fix this during landing if > you find the rest ok? Sure, it's just a nit, I didn't mean to r- this patch at all. (In reply to Carlos Garcia Campos from comment #4) > And smart pointers are not utilities? I find the rename confusing and > unnecessary, but I won't oppose if it makes sense for others. Smart pointers are utilities but if you use a smart pointer without the proper deleter/ref/unref declared, you get trouble. For that you need to declare the proper deleter/ref/unref that happens to be in the G*PtrGStreamer.h files. I want to give the impression that is file is not just about utilities that you can use or not, it is something that you should include. That's why went thru the pain of changing the name. Created attachment 337872 [details]
Patch
Style fix
Comment on attachment 337872 [details] Patch Clearing flags on attachment: 337872 Committed r230625: <https://trac.webkit.org/changeset/230625> All reviewed patches have been landed. Closing bug. |