Code review comment for lp:~michihenning/unity-scopes-api/config-fixes

Thanks heaps for the thorough review!

> - scoperegistry, scoperunner, and smartscopesproxy are now installed in
> /usr/lib/<arch> (instead of a subdir of /usr/lib/arch).
>
> Why this change? There is loads of stuff in the /usr/lib/arch/, we should keep
> some namespacing - at least a ../arch/unity-scopes/ dir.

I agree. It's /usr/lib/<arch>/unity-scopes now.

> 77 + The default value is "$XDG_RUNTIME_DIR/<effective UID>/zmq". If
> XDG_RUNTIME_DIR is not
> 78 + set, "/run/user/<effective UID>/zmq" is used.
>
> The fallback note seems out-of-date, plus $XDG_RUNTIME_DIR already includes
> UID.

Fixed, thanks!

> 80 +- EndpointDir.Private (string)
>
> Can you remind you why do we even have this? Scopes will only have access to
> their own endpoint directory and aggregating scopes (+registry, +dash) need to
> access them all, so isn't this option obsolete?

That's a hangover from the days where I wasn't sure how we would secure access to the endpoints. I agree, we can hardwire the priv subdir.

Basically, public endpoints go into $XDG_RUNTIME_DIR/zmq, and private ones into $XDG_RUNTIME_DIR/zmq/priv.

Fixed.

> 111 + The default value is
> "@CMAKE_INSTALL_PREFIX@/@SCOPES_DEFAULT_CONFIGDIR@/scoperunner".
> 116 + The default value is
> "@CMAKE_INSTALL_PREFIX@/@SCOPES_DEFAULT_CONFIGDIR@/scopes".

That's a doc bug, thanks for that! The actual install location is under /usr/lib/<arch>

> Arch-specific binaries can't go into /usr/share/ (and SCOPES_DEFAULT_CONFIGDIR
> == /usr/share/unity-scopes-api). Moreover the default config files still point
> to @CMAKE_INSTALL_PREFIX@/@CMAKE_INSTALL_LIBDIR@/scoperunner/scoperunner and
> @CMAKE_INSTALL_PREFIX@/@CMAKE_INSTALL_LIBDIR@/unity-scopes.
>
> 121 + by OEMs. The default value is
> "@CMAKE_INSTALL_PREFIX@/custom/@LIB_INSTALL_PREFIX@/scopes".
>
> Shouldn't this be without the CMAKE_INSTALL_PREFIX prefix? :)
>
> 126 + The defaul value is "$HOME/.local/sahre/unity-scopes".
>
> Typos - "defaul", "sahre".

Fixed.

> I also noticed Registry.ini's endpoint option disappeared, is it now just
> Zmq.ini's EndpointDir.Public + "Registry"?

Yes. There is no point in having a separate config item for this.

> 422 - void run_scope(ScopeBase *const scope_base, std::string const&
> scope_ini_file);
> 423 + void run_scope(ScopeBase *const scope_base, std::string const&
> runtime_ini_file, std::string const& scope_ini_file);
>
> :/ Break in public API. How about overload instead?

Yes, that's better! Fixed, thanks!

> 477 +static constexpr char const* DFLT_SCOPERUNNER_PATH =
> "@CMAKE_INSTALL_PREFIX@/@SCOPES_DEFAULT_CONFIGDIR@/scoperunner";
> 478..484
>
> Please keep in sync with the default config files.

Yes! :-)

There is no need to install any config files now. Everything defaults to something sensible, so I've removed the installation of the config files.

I also removed an inconsistency: we used unity-scopes as a subdir for /usr/lib/<arch>, but unity-scopes-api as a subddir for /usr/share. They are both unity-scopes subdirs now.