I hope the below code review is ok. I have a lot of experience with Ansible, but not much experience with Fedora code reviews. I'm just reviewing what I can see from the patch - if anyone can point me at where I can actually see the repo, that would be helpful.
This looks like there could be a for loop in the template, rather than pretty much hardcoding the bot specification for each list.
The code pattern {% if env == "staging" %} is a bit of a red flag (mostly because it leads to problems later where you add a new non-prod env). Really you should be using group variables. So you might have a default bot_suffix of "", and then the staging group (or whichever group is responsible for setting env to staging) would have a bot_suffix of "-s"
Will
On Fri, Sep 2, 2016 at 1:14 PM, Justin W. Flory <jflory7@xxxxxxxxx> wrote:
Hi all,
This is my first time submitting a patch to a repo or during a freeze
break - if I missed a step or did something wrong, feel free to correct
me. :) I didn't see a SOP in infra-docs so I just followed the format of
past freeze break requests.
This is a quick patch to add a new bot to ircbot.py for the
#fedora-diversity team. I copied the commopswatch bot and made slight
modifications. But I would appreciate it were reviewed for accuracy in
case I missed something.
Patch is below and also attached to the email.
>From 1b182e380e10d978c4daa7f01d859916d5675b78 Mon Sep 17 00:00:00 2001
From: "Justin W. Flory" <git@xxxxxx>
Date: Thu, 1 Sep 2016 23:07:04 -0400
Subject: [PATCH] Add #fedora-diversity to ircbot.py for notifications
related
to diversity
---
roles/fedmsg/irc/templates/ircbot.py | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/roles/fedmsg/irc/templates/ircbot.py
b/roles/fedmsg/irc/templates/ircbot.py
index 2bb4c03..3f3931b 100644
--- a/roles/fedmsg/irc/templates/ircbot.py
+++ b/roles/fedmsg/irc/templates/ircbot.py
@@ -363,6 +363,27 @@ config = dict(
body=['^((?!(modularity|Modularity)).)*$'],
),
),
+
+ # And #fedora-diversity wanted in too
+ dict(
+ network='chat.freenode.net',
+ port=6667,
+ make_pretty=True,
+ make_terse=True,
+
+ {% if env == 'staging' %}
+ nickname='diversity-bot-s',
+ {% else %}
+ nickname='diversity-bot',
+ {% endif %}
+ channel='fedora-diversity',
+ filters=dict(
+ topic=[
+
'(planet|meetbot\.meeting\.item\.help|meetbot\.meeting| fedocal\.meeting\.new|fedocal\ .meeting\.update|fedocal\. meeting\.delete|fedocal\. calendar|fas\.group\.member\. sponsor|pagure\.project\.new| askbot\.post\.flag_offensive)' ,
+ ],
+ body=['^((?!diversity).)*$'],
+ ),
+ ),
],
### Possible colors are ###
--
2.7.4
--
Cheers,
Justin W. Flory
jflory7@xxxxxxxxx
_______________________________________________
infrastructure mailing list
infrastructure@lists.fedoraproject.org
https://lists.fedoraproject.org/admin/lists/ infrastructure@lists. fedoraproject.org
_______________________________________________ infrastructure mailing list infrastructure@xxxxxxxxxxxxxxxxxxxxxxx https://lists.fedoraproject.org/admin/lists/infrastructure@xxxxxxxxxxxxxxxxxxxxxxx
