Code review comment for lp:~ted/ubuntu-app-launch/app-store

Revision history for this message
Ted Gould (ted) wrote :

On Tue, 2017-03-14 at 03:55 +0000, Charles Kerr wrote:
> Inline are more minor cleanup suggestions. Most of them are in moved
> code rather than new code. The new code LGTM
So I'm going to ignore a bunch of these because they're in code that
gets deleted real soon, and I'm lazy :-)
> > +/** Looks for an application by looking through the system and user
> > + application directories to find the desktop file.
> > +
> > + \param package Container name
> > + \param appname Application name to look for
> > + \param registry persistent connections to use
> > +*/
> > +bool Legacy::verifyAppname(const AppID::Package& package,
> > + const AppID::AppName& appname,
> > + const std::shared_ptr<Registry>& registry)
> > +{
> > + if (!verifyPackage(package, registry))
> > + {
> > + throw std::runtime_error{"Invalid Legacy package: " + std::string(package)};
> > + }
> > +
> > + auto desktop = std::string(appname) + ".desktop";
> > + auto evaldir = [&desktop](const gchar* dir) -> bool {
> >

>
>
> "-> bool" should be unnecessary; there's only one return point so g++ should be able to deduce the return type
>

Fixed r303. But you can't have this an the "return TRUE" below :-)
> > + if (evaldir(g_get_user_data_dir()))
> > + {
> > + return true;
> > + }
> > +
> > + const char* const* data_dirs = g_get_system_data_dirs();
> >

>
> auto&& data_dirs = g_get_system_data_dirs()
>

r304, cool, wouldn't have guessed that would have worked :-)

« Back to merge proposal