Giuseppe Vallarelli has posted comments on this change.
Change subject: Added the hook dispatcher
......................................................................
Patch Set 2: I would prefer that you didn't submit this
(7 inline comments)
Except for the few trivial comments above, looks good to me.
....................................................
File hooks/hook-dispatcher
Line 25: from collections import OrderedDict
Line 26: from dulwich.repo import Repo
Line 27:
Line 28:
Line 29: def ignore(ignored_hooks, current_hooks, args):
You're not using args here, are you complying to a common interface ?
Line 30: """
Line 31: Return the hooks list without the given hooks
Line 32:
Line 33: :param ignored_hooks; csv lis tof the hooks to ignore
Line 29: def ignore(ignored_hooks, current_hooks, args):
Line 30: """
Line 31: Return the hooks list without the given hooks
Line 32:
Line 33: :param ignored_hooks; csv lis tof the hooks to ignore
typo (csv lis tof)
Line 34: :param current_hooks: array with the currently available hooks
Line 35: """
Line 36: ignored_hooks = set([hook.strip() for hook in
ignored_hooks.split(',')])
Line 37: logging.info(" Ignoring the hooks %s", ignored_hooks)
Line 32:
Line 33: :param ignored_hooks; csv lis tof the hooks to ignore
Line 34: :param current_hooks: array with the currently available hooks
Line 35: """
Line 36: ignored_hooks = set([hook.strip() for hook in
ignored_hooks.split(',')])
you may achieve the same result and less loc by creating 2 sets and then apply
the difference set_available - set_ignore.
Line 37: logging.info(" Ignoring the hooks %s", ignored_hooks)
Line 38: for hook in ignored_hooks:
Line 39: match = filter(lambda h: re.match(hook, h), current_hooks)
Line 40: if match:
Line 42: current_hooks.pop(current_hooks.index(mhook))
Line 43: return current_hooks
Line 44:
Line 45:
Line 46: def run_only(to_run_hooks, current_hooks, args):
see comment above for args in ignore.
Line 47: """
Line 48: Handles the Run-Only tag, removes all the hooks not macching the
given
Line 49: hooks
Line 50:
Line 44:
Line 45:
Line 46: def run_only(to_run_hooks, current_hooks, args):
Line 47: """
Line 48: Handles the Run-Only tag, removes all the hooks not macching the
given
typo (matching).
Line 49: hooks
Line 50:
Line 51: :param to_run_hooks: array with the csv string passed to the tag
Line 52: :param current_hooks: array with the current available hooks
Line 50:
Line 51: :param to_run_hooks: array with the csv string passed to the tag
Line 52: :param current_hooks: array with the current available hooks
Line 53: """
Line 54: to_run_hooks = set([hook.strip() for hook in
to_run_hooks.split(',')])
This is a common operation you may extract it in a function (set([...]) ).
Line 55: logging.info(" Running only the hooks %s", to_run_hooks)
Line 56: to_run = []
Line 57: for hook in to_run_hooks:
Line 58: if hook in current_hooks:
Line 109: rerun
Line 110: ),
Line 111: }
Line 112:
Line 113:
You may refactor get_commit_tags and get_comment_tags by exctracting common
code.
Line 114: def get_commit_tags(commit):
Line 115: """
Line 116: Get the tags that were specified in the comit message
Line 117:
--
To view, visit http://gerrit.ovirt.org/15386
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iada3d1bef5357366d5f319561efac8ee85a614f0
Gerrit-PatchSet: 2
Gerrit-Project: gerrit-admin
Gerrit-Branch: master
Gerrit-Owner: David Caro <dcaro...@redhat.com>
Gerrit-Reviewer: David Caro <dcaro...@redhat.com>
Gerrit-Reviewer: Eyal Edri <ee...@redhat.com>
Gerrit-Reviewer: Giuseppe Vallarelli <gvall...@redhat.com>
Gerrit-Reviewer: Itamar Heim <ih...@redhat.com>
Gerrit-Reviewer: Ohad Basan <oba...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
