[Merge] lp:~ssweeny/location-service/cleanups.15.04 into lp:location-service/15.04

Thomas Voß thomas.voss at canonical.com
Wed Mar 2 16:56:58 UTC 2016


Review: Needs Information

A suggestion inline, let me know what you think.

Diff comments:

> 
> === modified file 'src/location_service/com/ubuntu/location/service/daemon.h'
> --- src/location_service/com/ubuntu/location/service/daemon.h	2015-01-25 12:45:30 +0000
> +++ src/location_service/com/ubuntu/location/service/daemon.h	2016-02-01 21:58:38 +0000
> @@ -163,6 +164,9 @@
>      /** @brief Pretty-prints the CLI's help text to the given output stream. */
>      static void print_help(std::ostream& out);
>  
> +    /** @brief Instantiates and configures each provider selected in the config */
> +    static void load_providers(const Configuration& config, std::shared_ptr<location::Engine> engine);

I would propose that load_providers returns a std::set<Provider::Ptr>. With that, the function is free of side effects altering the engine and easier to test. The calling code could then just do for (auto provider : Daemon::load_providers(config)) { engine->add_provider(provider); }

> +
>      /**
>       * @brief Executes the daemon with the given configuration.
>       * @return EXIT_SUCCESS or EXIT_FAILURE.


-- 
https://code.launchpad.net/~ssweeny/location-service/cleanups.15.04/+merge/284676
Your team Ubuntu Phablet Team is subscribed to branch lp:location-service/15.04.



More information about the Ubuntu-reviews mailing list