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

Alexandre Abreu alexandre.abreu at canonical.com
Thu Jun 16 13:29:34 UTC 2016


> > 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,
> 
> I’m not sure I’m following you here. isPrintHelpLaunch() is not used outside
> of the context of BrowserApplication (and indeed it’s a member function), so
> what’s wrong with making it non-static and accessing member variables?
> It has a similar scope to BrowserApplication::inspectorPort() or
> BrowserApplication::urls(), which both read m_arguments.

yes but the key point is that it is a pure side effect free function
which I tend to set as static. Why make a function access its environment
where it does not need to be? and here it does not need to,

but I changed it to using m_arguments (even if I am not really for it) for
the sake of consistency w/ the rest, and it is a minor thing anyway,

> 
> > What bothers you is the double calls to isPrintHelpLaunch?
> 
> I guess the naming does :) Can it be renamed to e.g. "helpRequested", or maybe
> "shouldDisplayHelp"?

done to helpRequested,
-- 
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