Code review comment for lp:~lifeless/launchpad/lsprof

Revision history for this message
Curtis Hovey (sinzui) wrote :

Thanks for extracting the profiling behaviour. I have a few remarks.

> === modified file 'lib/canonical/launchpad/webapp/configure.zcml'
> --- lib/canonical/launchpad/webapp/configure.zcml 2010-06-08 15:57:09 +0000
> +++ lib/canonical/launchpad/webapp/configure.zcml 2010-06-27 09:23:29 +0000
> @@ -13,6 +13,8 @@
> <include file="errorlog.zcml" />
> -->
> <include file="bug-5133.zcml" />
> + <!-- Would be nice to turn this off in production -->
> + <include file="profile.zcml" />

Each env gets its own config. ./configs/testrunner-appserver has ZCML
that registers YUI unittest hooks for example. I think this rule could
be in ./configs/development, and in ~launchpad-pqm/production-configs in
staging and dogfood.

> === added file 'lib/canonical/launchpad/webapp/profile.py'
> --- lib/canonical/launchpad/webapp/profile.py 1970-01-01 00:00:00 +0000
> +++ lib/canonical/launchpad/webapp/profile.py 2010-06-27 09:23:29 +0000

We do not want to add to this deprecated location. I think this would
be better located at lib/lp/services/profile

> === added file 'lib/canonical/launchpad/webapp/profile.zcml'
> --- lib/canonical/launchpad/webapp/profile.zcml 1970-01-01 00:00:00 +0000
> +++ lib/canonical/launchpad/webapp/profile.zcml 2010-06-27 09:23:29 +0000

As above.

review: Needs Information

« Back to merge proposal