D17560: fully port away from kdelibs4support

Albert Astals Cid noreply at phabricator.kde.org
Thu Feb 21 18:15:02 GMT 2019


aacid added inline comments.

INLINE COMMENTS

> sitter wrote in toplevel.cpp:236
> Hm. That is true. Truth be told even after pointing that out it still took me way to long to see the problem -.-
> 
> I think I wanted to avoid the extra level of nesting. With that motivation in mind, what do you think about splitting the if
> 
>   if (soundEnabled && language.isEmpty())
>   {
>     const QStringList &systemLanguages = KLocalizedString::languages();
>     ....
>   }
>   
>   if (!soundEnabled)
>   {
>     soundOff();
>     language = QString();
>   }
> 
> In my mind the two code blocks aren't two sides of the same coin. One is language fallback-handling the other is the unrelated disabling of all sound. The only thing that links them is the fact that the only thing language is used for is to figure out what sound set to use.
> 
> I don't mind particularly though, so if you'd like to have
> 
>   if (soundEnabled) {
>     if (language.isEmpty()) { ... }
>   } else { ... }
> 
> I'm fine with that too

Not attached to any solution, just want one that works :)

REPOSITORY
  R418 KTuberling

REVISION DETAIL
  https://phabricator.kde.org/D17560

To: sitter, aacid, #kde_games
Cc: kde-games-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-games-devel/attachments/20190221/285135f8/attachment.html>


More information about the kde-games-devel mailing list