Ewoud Kohl van Wijngaarden has posted comments on this change.
Change subject: Add new job update-engine-params
......................................................................
Patch Set 13:
(5 comments)
I think this could be a great script. Just some suggestions to make it easier
to use in the future.
....................................................
File jobs/ovirt_engine_upgrade_params/update_engine_params.sh
Line 4: WORKSPACE=$1
Line 5: FROM=$2
Line 6: TO=$3
Line 7:
ANS_FILE_33="${WORKSPACE}"/jenkins/jobs/ovirt_engine_upgrade_params/answer.file.otopi
Line 8:
CLEANUP_FILE_33="${WORKSPACE}"/jenkins/jobs/ovirt_engine_upgrade_params/cleanup.file.otopi
Not sure if these should have 33 in their variable name while the files
themselves aren't versioned. If the files are named x.file.$VERSION.otopi, I
think it would be easier to test both 3.3 -> nightly and 3.4 -> nightly with
this script. Maybe even 3.3 -> 3.4.
Line 9:
STABLE_33_REPO="${WORKSPACE}"/jenkins/jobs/ovirt_engine_upgrade_params/ovirt_engine_stable.repo
Line 10:
NIGHTLY_33_REPO="${WORKSPACE}"/jenkins/jobs/ovirt_engine_upgrade_params/ovirt_engine_nightly.repo
Line 11: DATE=$(date +"%a_%b_%d_%H_%M_%S_%Z_%Y")
Line 12: LOG="${WORKSPACE}/${DATE}.log"
Line 6: TO=$3
Line 7:
ANS_FILE_33="${WORKSPACE}"/jenkins/jobs/ovirt_engine_upgrade_params/answer.file.otopi
Line 8:
CLEANUP_FILE_33="${WORKSPACE}"/jenkins/jobs/ovirt_engine_upgrade_params/cleanup.file.otopi
Line 9:
STABLE_33_REPO="${WORKSPACE}"/jenkins/jobs/ovirt_engine_upgrade_params/ovirt_engine_stable.repo
Line 10:
NIGHTLY_33_REPO="${WORKSPACE}"/jenkins/jobs/ovirt_engine_upgrade_params/ovirt_engine_nightly.repo
These two REPO vars are now unused.
Line 11: DATE=$(date +"%a_%b_%d_%H_%M_%S_%Z_%Y")
Line 12: LOG="${WORKSPACE}/${DATE}.log"
Line 13: HOSTNAME=$(hostname)
Line 14:
Line 69:
Line 70: configure_from_repo()
Line 71: {
Line 72: if [[ "${FROM}" == "stable" ]]; then
Line 73: cat << "EOF" > /etc/yum.repos.d/${FROM}.repo
I think this should be ovirt-${FROM}.repo.
Line 74: [ovirt-engine-stable]
Line 75: name=oVirt Engine Stable
Line 76: baseurl=http://resources.ovirt.org/releases/3.3/rpm/Fedora/$releasever/
Line 77: enabled=1
Line 72: if [[ "${FROM}" == "stable" ]]; then
Line 73: cat << "EOF" > /etc/yum.repos.d/${FROM}.repo
Line 74: [ovirt-engine-stable]
Line 75: name=oVirt Engine Stable
Line 76: baseurl=http://resources.ovirt.org/releases/3.3/rpm/Fedora/$releasever/
$releasever should be escaped. Also, why not use the stable symlink instead of
hardcoding 3.3? Better yet, just use $FROM in both the name and baseurl.
Line 77: enabled=1
Line 78: gpgcheck=0
Line 79: EOF
Line 80: fi
Line 108: copy_log
Line 109: }
Line 110:
Line 111:
Line 112: configure_to_repo()
Same comments as configure_from_repo. I think they even could be merged if you
use $1 instead of the global $TO.
Line 113: {
Line 114: if [[ "${TO}" == "nightly" ]]; then
Line 115: cat << "EOF" > /etc/yum.repos.d/${TO}.repo
Line 116: [ovirt_engine_nightly repo]
--
To view, visit http://gerrit.ovirt.org/21094
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1809bd55c88b17f1d7dfde3920212e6f046709a8
Gerrit-PatchSet: 13
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: Kiril Nesenko <knese...@redhat.com>
Gerrit-Reviewer: Ohad Basan <oba...@redhat.com>
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
