Review Request: Make it possible to restore presence on login

Sat Apr 7 15:26:14 UTC 2012

On April 5, 2012, 10 a.m., Dominik Cermak wrote:
> > Perhaps I didn't explain it correctly, I didn't mean to remove the enum completely, but just to read and write the "full" version
> > Anyway it's not a big deal it is not used much... but you should definitely have 3 const variables containing the string and use them instead of using QLatin1String(...) everywhere
> 
> Dominik Cermak wrote:
>     I'm pretty much confused...
>     AFAIK (but please correct me if I'm wrong) enums are integer only. To save it as a string in the config file I would need to do a conversion for reading and writing (enum -> string, string -> enum).
>     So using the enum as well as the strings only brings one more step:
>     
>     for enum only (save as int) or strings only:
>       1. read value from config
>       2. compare to decide what to to
>     
>     for strings and enum:
>       1. read value from config
>       2. convert to corresponding enum value
>       3. compare to decide what to do
> 
> Dominik Cermak wrote:
>     Anyway it missed feature freeze for 0.4 so maybe just discard this one and redo it for 0.5 (handling different settings per account, solving the "2 relogins needed" problem and so on)?

We are still in "soft freeze" that means, afaik, that rewiews pending can still be merged, so no problem in merging this if you finish it before the "hard freeze"

Yes, enums are "named" integer, inside the code it makes sense to use enums, but in the config file it does not make sense to me to save the integer, because the config file is supposed to be editable manually and an enum would make it difficult to understand

Another problem in doing that (even though it's quite unlikely to happen here) is that you have an enum
{ disabled, manual, enabled } you will have disabled=0 manual=1 enabled=2. If you save the number in your config file and then for some reason we decide that we need to add one or to reorder them, the values will change and when the user runs the program, the value read from the old config will be associated to the new value and therefore it will have another meaning...
With a string it takes some more steps, but you are sure that, no matter how you reorder the stuff inside, the associated value is always the same. Writing and reading config files is not a bottleneck where we have to optimize the code as much as we can, so I see no problem in doing it.

Anyway using just strings inside can be good as well, but just make them constant somewhere instead of writing them every time you use them, so that if we want to change the string, we don't have to do it everywere in the code, but just in one place. Moreover we could have other "manual" strings around the code, so a variable like autoLoginManual is a lot more expressive

- Daniele Elmo

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

On March 24, 2012, 10:27 p.m., Dominik Cermak wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104363/
> -----------------------------------------------------------
> 
> (Updated March 24, 2012, 10:27 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> It's a simple approach to have this feature implemented.
> It uses the ConnectsAutomatically property and the AutomaticPresence.
> In the kcm you can enable/disable this feature (ConnectsAutomatically is set) and on every presence change the new presence is set as the AutomaticPresence.
> This way mission-control cares for setting the saved presence as soon as possible.
> 
> Note: Disabling it takes only effect after the second login, I suspect something with the change notification between kcm and kded isn't working (that signal kcm should send and kded receive). So kded sets the property only after starting...
> 
> 
> This addresses bug 281929.
>     http://bugs.kde.org/show_bug.cgi?id=281929
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt c55fa8431464b62a5426a6cf29319c3f6f5fce6f 
>   autoconnect.h PRE-CREATION 
>   autoconnect.cpp PRE-CREATION 
>   config/telepathy-kded-config.cpp 1fb515f56500407b7b221d1d5310237f8d94ca7f 
>   config/telepathy-kded-config.ui 0d616c5503b22f37c0ae4d5a94411d0f97b25d38 
>   telepathy-module.h 05d8c3847587049dcc4c9329a3b4d0b0bfee7d49 
>   telepathy-module.cpp 955ceec3a8fdaacaf24463ac4e721d0a53086d30 
> 
> Diff: http://git.reviewboard.kde.org/r/104363/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dominik Cermak
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20120407/fa89b275/attachment-0001.html>