Review Request 120075: KArchive: add support for rcc files

[David Faure]

> On Oct. 11, 2014, 2:38 p.m., Milian Wolff wrote:
> > src/krcc.cpp, line 154
> > <https://git.reviewboard.kde.org/r/120075/diff/1/?file=310012#file310012line154>
> >
> >     maybe I'm missing something, but this looks like you don't need to override it at all when you just delegate to the base class anyways?
> 
> David Faure wrote:
>     The whole point is to be able to extend this later without requiring an app recompilation.
> 
> Milian Wolff wrote:
>     as I said, maybe I'm missing something, but here I come again:
>     
>     the base function is virtual, so it will always go through the vtable, no matter whether you added an overload or not. you'll only ever need to recompile karchive when you add functionality to it, and that you'll need to do no matter what since you'll need to add code to this function in such a case.

But you might need to use the hook for a bugfix, not only for a new feature.

And then without an empty reimplementation it wouldn't be called in the non-recompiled KRcc instances.

The virtual_hook tricky has always been done this way, btw ;-)


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120075/#review68263
-----------------------------------------------------------


On Oct. 11, 2014, 3:37 p.m., David Faure wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120075/
> -----------------------------------------------------------
> 
> (Updated Oct. 11, 2014, 3:37 p.m.)
> 
> 
> Review request for KDE Frameworks, Mario Bensi and Kevin Krammer.
> 
> 
> Repository: karchive
> 
> 
> Description
> -------
> 
> These the dynamic binary resources created by Qt's rcc tool from a .qrc file
> and the files it points to.
> 
> [ChangeLog] KArchive: add support for rcc files
> 
> REVIEW: 120075
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt 948a25b5e017a11c1577c7c324f2fa3545b13f52 
>   src/krcc.h PRE-CREATION 
>   src/krcc.cpp PRE-CREATION 
>   tests/CMakeLists.txt ebcd01f5bfdec1298bbc936b9429b187f50c9b45 
>   tests/krcctest.cpp PRE-CREATION 
>   autotests/karchivetest.h 567be075758f6afd4bdff5a2f6f8ef5e392a53eb 
>   autotests/karchivetest.cpp 6b50f32a6d99756ba246262067dd71573f129256 
>   autotests/runtime_resource.rcc PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/120075/diff/
> 
> 
> Testing
> -------
> 
> Manual test prog, autotest.
> 
> 
> Thanks,
> 
> David Faure
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20141011/23680106/attachment.html>