Dan Kenigsberg has posted comments on this change.
Change subject: Add hooksTests. ......................................................................
Patch Set 1: I would prefer that you didn't submit this
(5 inline comments)
.................................................... File tests/hooksTests.py Line 35: echo -n %s $1 >> "$_hook_domxml" "$1" should be quoted too. (just for good practice, here it means very little)
hmm, why do you have it here at all?
Line 58: dirName = tempfile.mkdtemp(dir="/tmp") why do you care that dir="/tmp"? mkdtemp does the Right Thing on its own.
Line 62: os.rmdir(dirName) you can easily add a test for hooks._scriptsPerDir
.................................................... File vdsm/hooks.py Line 34: if (dir[0]=='/'): wrap with spaces, to make pep8 happy
Line 41: def _runHooksDir(domxml, dir, vmconf={}, raiseError=True): please add a docstring about the funny new sematics of "dir" (a leading / means absolute path, anything else is relative to P_VDSM_HOOKS)
-- To view, visit http://gerrit.ovirt.org/3589 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ic4e9c5acc4ae8a1fa352c7cec4724a930c837257 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan ykaplan@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com