On Tue, Sep 24, 2013 at 09:47:57AM +0200, Miroslav Suchy wrote:
I like it. With few comments:
Great! I've revised the patch in-place at:
http://threebean.org/patches/copr-backend-first-pass-at-adding-fedmsg-hoo...
* variable "what" is now undefined. It was parameter of
event(), you removed
it, but you continue to use it.
Ah, I actually define it inside the event function now:
+ content = content or {}
+ what = template.format(**content)
* can you please add docstring to event(), which will explain the
params?
Added.
* I would much rather see it driven by config. I.e.have in
copr-be.conf:
fedmsg_enabled=false
as default. And in this case use such code:
opts.fedmsg_enabled = _get_conf(cp, 'backend', 'fedmsg_enabled', False)
...
try:
if opts.fedmsg_enabled:
fedmsg.init(name="relay_inbound", cert_prefix="copr",
active=True)
which will neither fail nor emit log entries when fedmsg is not configured
(by purpose).
Good idea! This has been added everywhere (and an example added to
copr-be.conf.example).
* can you please add the new requires to copr.spec in root of
copr.git?
Done!