Review Request 109268: Combining settings in a singleton class so that there is no confusion about the current settings.

[Stefan Eggers]

> On March 3, 2013, 9:09 p.m., Lasath Fernando wrote:
> > lib/text-chat-config.cpp, line 42
> > <http://git.reviewboard.kde.org/r/109268/diff/1/?file=116772#file116772line42>
> >
> >     I think you should probably put a mutex lock around this.

Wouldn't I then have to use a mutex in all of the methods? I think the accesses to the members of TextChatConfig are not guaranteed to be atomically.

If it is deemed necessary I would then add a class variable of type QMutex (using QMutex::Recursive) to TextChatConfig and use that in all methods.


- Stefan


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


On March 3, 2013, 8:44 p.m., Stefan Eggers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109268/
> -----------------------------------------------------------
> 
> (Updated March 3, 2013, 8:44 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> Combining settings in a singleton class so that there is no confusion about the current settings.
> 
> Up to now the settings for where new chats open were loaded by
> ktp-text-ui on startup and not re-read anymore. Changing the settings
> in the config dialog had no immediate effect; only after the next
> start of ktp-text-ui it finally changed.
> 
> Now whenever the settings get asked for the singleton provides the
> current value. Thus every change to the settings via the config dialog
> will take effect immediately.
> 
> Also gives a way to easily add new settings in one place. Will be
> useful for implementing the additional settings required by bug 282201
> (make it configurable if "user is typing" gets show) which needs two
> more bools.
> 
> 
> This addresses bug 316004.
>     http://bugs.kde.org/show_bug.cgi?id=316004
> 
> 
> Diffs
> -----
> 
>   config/behavior-config.h 562187ee308182f70b66b09ecf489a13fbfb6fdd 
>   app/telepathy-chat-ui.cpp 920210891e58166d1c14f5431a913a8e5f912de2 
>   app/telepathy-chat-ui.h 70800f42a4dd8f5627495661a2f9c71535f20763 
>   config/behavior-config.cpp 214ee3e5641185d200c9aca2892aee78e9915e5b 
>   lib/CMakeLists.txt 4e03671dcdaee338aca57dae12512b75d7f0be15 
>   lib/KTp/TextChatConfig PRE-CREATION 
>   lib/text-chat-config.h PRE-CREATION 
>   lib/text-chat-config.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/109268/diff/
> 
> 
> Testing
> -------
> 
> 1. Starting with "Open new conversations:" set to "As tabs in the same window".
> 2. Open a chat with user A.
> 3. Open a chat with user B.
> 4. Observe that it opens in a new tab.
> 5. Change setting to "As new windows".
> 6. Close chat with user B.
> 7. Open a chat with user B.
> 8. Observe that it opens in a new window.
> 
> 
> Thanks,
> 
> Stefan Eggers
> 
>

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