Code review comment for lp://qastaging/~mterry/unity-scopes-api/snap-root

> Is $SNAP always going to have a trailing slash? If not, constructs such as
> [snip[
> won't create the path name correctly. Wouldn't it be safer to append a slash
> regardless? If we end up with two adjacent slashes, they won't do any harm.

@CMAKE_INSTALL_PREFIX@ is guaranteed to either be empty or start with a slash, right? So that's fine. And base_dflt_file always starts with a slash. So I think we're fine there.

> If SNAP isn't set, getenv() returns a nullptr, which causes the string
> constructor to throw. That causes a bunch of tests to fail.

Ick, right you are. I wrote so many of these in a row, I got confused between QString and std::string.

> It would be better to do this is ConfigBase and and provide a protected
> accessor for the value. That puts the code to sanitize the value in a single
> place.

OK.

> Here is a diff against this branch that makes some changes. Could you review please?

I would go make your suggested changes, but it sounds like you were trying to upload a diff to make them for me? I don't see one. I don't think LP MPs can have attachments.