Hi Will, On 09/02/2016 03:15 AM, Will Thames wrote: > 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. > You can find the source code for the file I'm patching here: https://infrastructure.fedoraproject.org/cgit/ansible.git/tree/roles/fedmsg/irc/templates/ircbot.py > > 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 > Not sure if this is an issue with my patch or the file as a whole. Let me know if this clarifies the patch. > On Fri, Sep 2, 2016 at 1:14 PM, Justin W. Flory <jflory7@xxxxxxxxx > <mailto: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 <mailto: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 <http://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 <mailto:jflory7@xxxxxxxxx> > > _______________________________________________ > infrastructure mailing list > infrastructure@xxxxxxxxxxxxxxxxxxxxxxx > <mailto:infrastructure@xxxxxxxxxxxxxxxxxxxxxxx> > https://lists.fedoraproject.org/admin/lists/infrastructure@xxxxxxxxxxxxxxxxxxxxxxx > <https://lists.fedoraproject.org/admin/lists/infrastructure@xxxxxxxxxxxxxxxxxxxxxxx> > > > > > _______________________________________________ > infrastructure mailing list > infrastructure@xxxxxxxxxxxxxxxxxxxxxxx > https://lists.fedoraproject.org/admin/lists/infrastructure@xxxxxxxxxxxxxxxxxxxxxxx > -- Cheers, Justin W. Flory jflory7@xxxxxxxxx
Attachment:
signature.asc
Description: OpenPGP digital signature
_______________________________________________ infrastructure mailing list infrastructure@xxxxxxxxxxxxxxxxxxxxxxx https://lists.fedoraproject.org/admin/lists/infrastructure@xxxxxxxxxxxxxxxxxxxxxxx
