[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