launchpad-dev team mailing list archive

[Aaron Bentley <aaron@xxxxxxxxxxxxx>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 09/21/2010 06:09 PM, Robert Collins wrote:
> yes, see the existing tests that do it ;) I suggest the memcache ones
> as an example. There is an XXX there that needs fixing too.

You mean this?

    >>> ignore = getFeatureStore().add(FeatureFlag(
    ...     scope=u'default', flag=u'memcache', value=u'disabled',
    ...     priority=1))
    >>> empty_request = LaunchpadTestRequest()
    >>> per_thread.features = FeatureController(
    ...     ScopesFromRequest(empty_request).lookup)

I think there's a lot of room for improvement here.  If feature flags
are meant to be pervasive, I think we should be able to depend on their
existence in tests, not need to create them explicitly-- otherwise,
we'll get test failures because the FeatureController hasn't been
initialized.  (Or worse, we'll accidentally leak FeatureControllers into
other tests.)  If performance is an issue, we can make a
LazyFeatureController that won't cost anything unless we use a FeatureFlag.

There's also a significant disconnect between how flags are queried and
how they are set.  Since they are queried via FeatureController, I
searched in vain for a method to set them there.

Also, features.getFeatureFlag is a function, not a method, so it should
not have a camelCase name.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkyjhJcACgkQ0F+nu1YWqI3k9QCfbILJTc+ny/BVsn2cHCchlxjV
3OEAn1sgKvJVrObQWyHFzfGXTs7ZSMxa
=1OBY
-----END PGP SIGNATURE-----