[Merge] lp:~abreu-alexandre/webbrowser-app/container-print-help-when-no-appid into lp:webbrowser-app

Alexandre Abreu alexandre.abreu at canonical.com
Tue Jun 14 13:19:54 UTC 2016


Thank you for your comments,

> BrowserApplication::isPrintHelpLaunch() doesn’t need to take an argument, it
> could be made non-static and read m_arguments.

I know, but I tend to prefer environment agnostic function when possible,
hence my approach there, I dont like "introspective" function that dont have to be,

> But there’s a cleaner approach I think: the check for -h or --help and the
> call to printUsage() could be done in the constructor
> (BrowserApplication::BrowserApplication()), and this would set a flag (maybe
> called "m_shouldExitEarly", or something like that) that the main function
> would test before calling initialize().

I am not sure if it is a cleaner approach, constructor work + state flag seem weird
to me,
What bothers you is the double calls to isPrintHelpLaunch?

> On a slightly related note, in WebappContainer::initialize() the call to
> earlyEnvironment() should go after the check for the appId.

+1, done
-- 
https://code.launchpad.net/~abreu-alexandre/webbrowser-app/container-print-help-when-no-appid/+merge/297220
Your team Ubuntu Phablet Team is requested to review the proposed merge of lp:~abreu-alexandre/webbrowser-app/container-print-help-when-no-appid into lp:webbrowser-app.



More information about the Ubuntu-reviews mailing list