Yaniv Bronhaim has posted comments on this change. Change subject: vdsm-infra: zombie-reaper refactor ......................................................................
Patch Set 5: Code-Review-1 (5 comments) http://gerrit.ovirt.org/#/c/31301/5//COMMIT_MSG Commit Message: Line 11: Line 12: It uses zombie-reaper as a sample use case. Line 13: Also removes the deprecated documentation in /doc/ and adds Line 14: documentation for zombie-reaper. Line 15: you added rst file kina empty.. so its not really a documentation for zombie-reaper.. and you removed the deprectaed files in a patch that already merged. please remove it from the commit msg. Line 16: Change-Id: I86a7d8d2f512395c2f50e21430b9e939f553e91b http://gerrit.ovirt.org/#/c/31301/5/configure.ac File configure.ac: Line 133: AC_SUBST([vdsmtsdir], ['${sysconfdir}/pki/vdsm']) Line 134: AC_SUBST([vdsmrepo], ['/rhev/data-center']) Line 135: AC_SUBST([vdsmpylibdir], ['${pythondir}/vdsm']) Line 136: AC_SUBST([vdsminfradir], ['${vdsmpylibdir}/infra']) Line 137: AC_SUBST([zombiereaperdir], ['${vdsminfradir}/zombiereaper']) imho we don't need to configure default path to a folder under infra, as we don't do it for folders under vdsmpylibdir Line 138: AC_SUBST([vdsmtooldir], ['${vdsmpylibdir}/tool']) Line 139: AC_SUBST([configuratorsdir], ['${vdsmtooldir}/configurators']) Line 140: AC_SUBST([vdsmtestsdir], ['${datarootdir}/vdsm/tests']) Line 141: http://gerrit.ovirt.org/#/c/31301/5/lib/vdsm/infra/__init__.py File lib/vdsm/infra/__init__.py: Line 12: # GNU General Public License for more details. Line 13: # Line 14: # You should have received a copy of the GNU General Public License Line 15: # along with this program; if not, write to the Free Software Line 16: # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA you can remove this glitch Line 17: # Line 18: # Refer to the README and COPYING files for full details of the license Line 19: # http://gerrit.ovirt.org/#/c/31301/5/lib/vdsm/infra/zombiereaper/Makefile.am File lib/vdsm/infra/zombiereaper/Makefile.am: Line 26: tests.py \ Line 27: $(NULL) Line 28: Line 29: check-local: Line 30: nosetests tests.py will each module keep a tests.py file under its tree? or better to move the tests under the main tests folder? http://gerrit.ovirt.org/#/c/31301/5/vdsm.spec.in File vdsm.spec.in: Line 129: Requires: python-argparse Line 130: Requires: python-cpopen >= 1.3 Line 131: Requires: python-ioprocess >= 0.12 Line 132: Requires: python-pthreading >= 0.1.3-3 Line 133: Requires: python-ethtool >= 0.6-3 is it intended and related? can be in separate patch with more details ? Line 134: Requires: %{name}-infra = %{version}-%{release} Line 135: Requires: rpm-python Line 136: Requires: nfs-utils Line 137: Requires: m2crypto -- To view, visit http://gerrit.ovirt.org/31301 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I86a7d8d2f512395c2f50e21430b9e939f553e91b Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi <smizr...@redhat.com> Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Dima Kuznetsov <dkuzn...@redhat.com> Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Saggi Mizrahi <smizr...@redhat.com> Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: Yeela Kaplan <ykap...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer <mta...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
