[Kde-pim] Review Request 110304: Remove dead code in VCardLine

[Milian Wolff]

> On May 4, 2013, 6:59 p.m., Kevin Krammer wrote:
> > Doesn't that change the size of VCardLine?
> 
> Milian Wolff wrote:
>     Yes, thats the whole point. VCardLine is not exported though so it should not matter.
> 
> Kevin Krammer wrote:
>     Are you sure? I checked the CMakeLists.txt of the directory and it installs vcardline.h and it is a .h and not a _p.h so I think it is public API.
>     Maybe the reason d is currently not used is that its need got removed but itself could not due to BC rules?
> 
> Sergio Luis Martins wrote:
>     It's missing the export macro though.
> 
> Kevin Krammer wrote:
>     Yes, but that only affects build that have visibilty-hidden enabled, no?
>     KDE does not ship any binary packages yet we still commit to BC within a major release. I don't think we can simply rule out one option for of building our code

Without the export macro you cannot use it from a different compile unit. Afaik KDE's cmake setup also ensures that this fails even on Linux, and not only on platforms where this is commonly enabled (like Windows).

Anyhow, discarded it now.


- Milian


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


On May 4, 2013, 8:26 p.m., Milian Wolff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110304/
> -----------------------------------------------------------
> 
> (Updated May 4, 2013, 8:26 p.m.)
> 
> 
> Review request for KDEPIM-Libraries.
> 
> 
> Description
> -------
> 
> The private class is not used and the pointer just increases the object size by 8bytes on a 64bit machine.
> 
> 
> Diffs
> -----
> 
>   kabc/vcardparser/vcardline.h 0cdf52b 
>   kabc/vcardparser/vcardline.cpp 349614e 
> 
> Diff: http://git.reviewboard.kde.org/r/110304/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Milian Wolff
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/