David Caro has posted comments on this change. Change subject: Passed pep8 and pyflakes. Use a global variable for the ssh command + common options. Updated all the email addresses as given in Code-review-1 Use argparse for command line options. Print return code, error and output on failures. Signed-off-by: Vishnu S ......................................................................
Patch Set 3: (2 comments) A couple comments inline. Have you been able to test it? http://gerrit.ovirt.org/#/c/37426/3/scripts/alert_old_patches.py File scripts/alert_old_patches.py: Line 101: SSH = "ssh -o StrictHostKeyChecking=no \ Line 102: -o UserKnownHostsFile=/dev/null \ Line 103: -i %s -p %s %s@%s" % ( Line 104: args.key, args.port, Line 105: args.user, args.server) Actually, I meant something more like creating the SSH global var in the top of the file, after the imports, and just appending the extra options if needed. Line 106: else: Line 107: SSH = "ssh -o StrictHostKeyChecking=no \ Line 108: -o UserKnownHostsFile=/dev/null \ Line 109: -p %s %s@%s" % (args.port, args.user, args.server) Line 136: txt = template % patch Line 137: msg = MIMEText('Subject: %s\n\n%s' % (subject, txt)) Line 138: msg['To'] = owner['email'] Line 139: if "@redhat.com" not in owner['email']: Line 140: msg['CC'] = "ih...@redhat.com,bazu...@redhat.com" Maybe being able to pass this as a parameter would be better, in case someone has to be added or removed, and for testing Line 141: mailserver.sendmail( Line 142: fromaddr, owner['email'], Line 143: msg.as_string()) Line 144: mailserver.quit() -- To view, visit http://gerrit.ovirt.org/37426 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic911411a61e097c734d0199cc04057c393b974f9 Gerrit-PatchSet: 3 Gerrit-Project: jenkins Gerrit-Branch: master Gerrit-Owner: Vishnu Sreekumar <vishnu.sr...@gmail.com> Gerrit-Reviewer: Barak Korren <bkor...@redhat.com> Gerrit-Reviewer: David Caro <dcaro...@redhat.com> Gerrit-Reviewer: Eyal Edri <ee...@redhat.com> Gerrit-Reviewer: Vishnu Sreekumar <vishnu.sr...@gmail.com> Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
