Ewoud Kohl van Wijngaarden has posted comments on this change. Change subject: Add generic code to check repoclosure ......................................................................
Patch Set 3: Code-Review-1 (6 comments) The indenting should be fixed, others are mostly questions. http://gerrit.ovirt.org/#/c/24802/3/jobs/packaging/repo_closure_check.sh File jobs/packaging/repo_closure_check.sh: Line 63: if [ "${EL6}" != "true" ]; then Line 64: echo "SKIPPING RUN: EL6 not checked in the configuration" Line 65: exit 0 Line 66: fi Line 67: custom_url="${BASE_URL}/${REPO_NAME}/rpm/${el6}" Odd indenting here Line 68: repoclosure \ Line 69: -t \ Line 70: --repofrompath=check-custom,"${custom_url}" \ Line 71: --repofrompath=check-base,http://centos.mirror.constant.com/6/os/x86_64/ \ Line 78: --repofrompath=check-glusterfs-epel-noarch,http://download.gluster.org/pub/gluster/glusterfs/LATEST/EPEL.repo/epel-6.4/noarch \ Line 79: -l check-updates \ Line 80: -l check-extras \ Line 81: -l check-epel \ Line 82: -l check-epel-testing \ By adding testing here, aren't we requiring users to enable epel-testing? Line 83: -l check-glusterfs-epel \ Line 84: -l check-glusterfs-noarch-epel\ Line 85: -l check-base \ Line 86: -l check-base-i386 \ Line 85: -l check-base \ Line 86: -l check-base-i386 \ Line 87: -r check-custom Line 88: ;; Line 89: fedora19) I wonder if we should make it more generic and make a check_closure_fedora function that takes a version, but for now that's not needed IMHO since every version may change. Line 90: if [ "${F19}" != "true" ]; then Line 91: echo "SKIPPING RUN: F19 not checked in the configuration" Line 92: exit 0 Line 93: fi Line 92: exit 0 Line 93: fi Line 94: custom_url="${BASE_URL}/${REPO_NAME}/rpm/${f19}" Line 95: repoclosure \ Line 96: -t \ Odd indenting here Line 97: --repofrompath=check-custom,"${custom_url}" \ Line 98: --repofrompath=check-fedora,http://mirrors.kernel.org/fedora/releases/19/Everything/x86_64/os/ \ Line 99: --repofrompath=check-updates,http://mirrors.servercentral.net/fedora/updates/19/x86_64/ \ Line 100: --repofrompath=check-updates-testing,http://mirrors.servercentral.net/fedora/updates/testing/19/x86_64/ \ Line 99: --repofrompath=check-updates,http://mirrors.servercentral.net/fedora/updates/19/x86_64/ \ Line 100: --repofrompath=check-updates-testing,http://mirrors.servercentral.net/fedora/updates/testing/19/x86_64/ \ Line 101: -l check-fedora \ Line 102: -l check-updates \ Line 103: -l check-updates-testing \ By adding testing here, aren't we requiring users to enable updates-testing? Line 104: -r check-custom Line 105: ;; Line 106: fedora20) Line 107: if [ "${F20}" != "true" ]; then Line 120: check By adding testing here, aren't we essentially requiring users to enable updates-testing? -- To view, visit http://gerrit.ovirt.org/24802 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I762abf97c9020301a388f7d83c7d1e41bdf1f062 Gerrit-PatchSet: 3 Gerrit-Project: jenkins Gerrit-Branch: master Gerrit-Owner: Kiril Nesenko <knese...@redhat.com> Gerrit-Reviewer: David Caro <dcaro...@redhat.com> Gerrit-Reviewer: Ewoud Kohl van Wijngaarden <ew...@kohlvanwijngaarden.nl> Gerrit-Reviewer: Eyal Edri <ee...@redhat.com> Gerrit-Reviewer: Ohad Basan <oba...@redhat.com> Gerrit-Reviewer: Sandro Bonazzola <sbona...@redhat.com> Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
