Code review comment for lp:~michihenning/unity-scopes-api/scope-cache-dir

Revision history for this message
Michi Henning (michihenning) wrote :

> I really don't like these changes, other than tests there's absolutely no
> reason for these to be virtual and therefore to allow the scope to override
> them.

The testing frameworks needs to allow a scope to be mocked, so it's not just
our tests. But I agree, the scope author should not be able to override these.

> Can't we do something different, like setting a special envvar for the
> tests and have the impl look at that first? Yes, making it virtual does make
> it easily unit-testable, but imo such a test is really just testing itself.

No more env vars! I'm going to institute a blanket ban on them. The are worse
than global variables: not declared anywhere totally invisible.

I've made the relevant methods final in ScopeBase, so it's no longer possible
to override them.

« Back to merge proposal