[Merge] lp:~phablet-team/messaging-app/messaging_app-fullscreen_flag into lp:messaging-app

Gustavo Pichorim Boiko gustavo.boiko at canonical.com
Tue Mar 22 17:45:12 UTC 2016


Review: Needs Fixing

Just a couple comments

Diff comments:

> === modified file 'src/messagingapplication.cpp'
> --- src/messagingapplication.cpp	2016-02-12 22:30:53 +0000
> +++ src/messagingapplication.cpp	2016-02-12 22:30:53 +0000
> @@ -92,7 +92,8 @@
>  
>  bool MessagingApplication::fullscreen() const
>  {
> -    return m_view->windowState() == Qt::WindowFullScreen;
> +    // FIXME: Correct flag to be used will be defined in future releases
> +    return m_view->flags() & static_cast <Qt::WindowFlags> (0x00800000);

So two things:
- Can you mention in the comment the name of the new flag that will be defined?
- Instead of having the hex value in different places, could you #define MESSAGING_FULLSCREEN_FLAG 0x00800000 and use it in all places needed?

>  }
>  
>  bool MessagingApplication::setup()


-- 
https://code.launchpad.net/~phablet-team/messaging-app/messaging_app-fullscreen_flag/+merge/285948
Your team Ubuntu Phablet Team is subscribed to branch lp:~phablet-team/messaging-app/side_panel.



More information about the Ubuntu-reviews mailing list