Tomas Golembiovsky has posted comments on this change. Change subject: v2v: Add execCmd() that accepts stdin/out/err ......................................................................
Patch Set 2: (2 comments) https://gerrit.ovirt.org/#/c/62092/2//COMMIT_MSG Commit Message: Line 3: AuthorDate: 2016-08-08 14:23:25 +0200 Line 4: Commit: Tomáš Golembiovský <tgole...@redhat.com> Line 5: CommitDate: 2016-08-09 16:21:43 +0200 Line 6: Line 7: v2v: Add execCmd() that accepts stdin/out/err > what about: v2v: add simple_exec_cmd that... Tried that. It's too long. :) If we're not too strict about the 50 character limit on the short description I can change that. Line 8: Line 9: There is no way to pass stdin, stdout and stderr streams to Popen via Line 10: execCmd. This will be useful in subsequent changes introducing logging Line 11: of virt-v2v output. Line 7: v2v: Add execCmd() that accepts stdin/out/err Line 8: Line 9: There is no way to pass stdin, stdout and stderr streams to Popen via Line 10: execCmd. This will be useful in subsequent changes introducing logging Line 11: of virt-v2v output. > In the end we are duplicating execCmd. I don't have a better alternative, s Not really. IMO the only "feature" of execCmd we duplicate is the call to cmdutils.wrap_command(). Line 12: Line 13: Change-Id: I6c748155e233ebfb2c0c223308693ccaab0ebc18 Line 14: Bug-Url: https://bugzilla.redhat.com/1350465 -- To view, visit https://gerrit.ovirt.org/62092 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6c748155e233ebfb2c0c223308693ccaab0ebc18 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky <tgole...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Milan Zamazal <mzama...@redhat.com> Gerrit-Reviewer: Shahar Havivi <shav...@redhat.com> Gerrit-Reviewer: Tomas Golembiovsky <tgole...@redhat.com> Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
