[Nepomuk] Review Request: Remove the Thing class

[Vishesh Handa]

> On July 28, 2012, 12:36 p.m., Vishesh Handa wrote:
> > I have just been informed that the tarballs for nepomuk-core have already been spun. Could we please please re-spin them, and include this patch. I don't care about the rest. They are just bug fixes.
> > 
> > This one removes an entire class, and if KDE 4.9, goes out without this patch, I can never remove it. Not until Frameworks.
> 
> Albert Astals Cid wrote:
>     What about things like kde-runtime/nepomuk/kioslaves/nepomuk/kio_nepomuk.cpp ?
> 
> Vishesh Handa wrote:
>     We just need to remove the headers includes.
>     
>     If it's too much of a problem, then it's fine. I'll deprecate the class in master (or just let it be), and live with it.
> 
> Albert Astals Cid wrote:
>     Do you have a list of all the tarballs that need to be remade?
> 
> Vishesh Handa wrote:
>     nepomuk-core and kde-runtime. 
>     
>     You'll anyway have to remake nepomuk-core for the windows stuff (if we want, otherwise they can just patch it themselves).
> 
> Albert Astals Cid wrote:
>     What about http://lxr.kde.org/source/playground/network/telepathy/ktp-nepomuk-service/nepomuk-storage.cpp#36 http://lxr.kde.org/source/playground/network/telepathy/ktp-nepomuk-service/tests/storage-test.cpp#41 http://lxr.kde.org/source/playground/network/telepathy/ktp-nepomuk-service/tests/tid-base-test.cpp#29 http://lxr.kde.org/source/playground/pim/zanshin/src/itemview/itemcontext.cpp#47 http://lxr.kde.org/source/playground/pim/zanshin/src/itemview/nepomukcontextmodel.cpp#36 http://lxr.kde.org/source/playground/pim/zanshin/src/utils/queries.cpp#30 http://lxr.kde.org/source/playground/pim/zanshin/src/utils/tagmanager.h#32
>     
>     Are we fine breaking them?
>     
>     Also can you guess-estimate the maintaince burden of this class being present during 4.9.x? Do you forsee it breaking/causing problems/making other fixes difficult to do?

The Telepathy one is my code, so I'm fine with breaking it. And I'm positive Christian (zanshin) won't mind. I'll patch up his code myself.

The main problem with this class is that it conflicts with the auto-generated classes - https://bugs.kde.org/show_bug.cgi?id=232513. Not removing it would mean finding some way to avoid the clash. Either the put the generated classes in a different namespace or something. I'm not sure how I'd fix it.

In terms of maintenance effort. It's nothing. That class will just lie there. It does virtually nothing.


- Vishesh


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105755/#review16568
-----------------------------------------------------------


On July 27, 2012, 2:37 p.m., Vishesh Handa wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105755/
> -----------------------------------------------------------
> 
> (Updated July 27, 2012, 2:37 p.m.)
> 
> 
> Review request for Nepomuk, Release Team and Sebastian Trueg.
> 
> 
> Description
> -------
> 
> No one uses it, and the same class can be generated using the rcgen. 
> I don't see the point of having this class.
> 
> This is my last chance to remove this old code. Otherwise I'll have to wait till KDE Frameworks 5.
> 
> 
> Diffs
> -----
> 
>   includes/CMakeLists.txt 4ac2d7c 
>   includes/Thing 952544e 
>   libnepomukcore/CMakeLists.txt 066c898 
>   libnepomukcore/resource/thing.h 2ae3d75 
>   libnepomukcore/resource/thing.cpp 59f2a4d 
> 
> Diff: http://git.reviewboard.kde.org/r/105755/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Vishesh Handa
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/nepomuk/attachments/20120730/3299afe4/attachment-0001.html>