[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