David Caro has posted comments on this change. Change subject: reorg repoclosure + tests ......................................................................
Patch Set 22: Code-Review-1 (10 comments) https://gerrit.ovirt.org/#/c/39249/22/jobs/packaging/repo_closure_check.sh File jobs/packaging/repo_closure_check.sh: Line 27: ) Line 28: # TEST_RUN is used by test_* script. it avoids running main, thus making Line 29: # possible to test functions defined in this file. Line 30: # another side effect is: die() is becoming undying - for testing purposes Line 31: [[ -z "${TEST_RUN}" ]] && TEST_RUN=0 use var="${var:-default}", now that you are moving the declarations here Line 32: declare -a MY_TMP_TRASH Line 33: declare BASE_URL Line 34: declare CENTOS_MIRROR Line 35: declare EPEL_MIRROR Line 35: declare EPEL_MIRROR Line 36: declare FEDORA_MIRROR Line 37: declare GLUSTER_MIRROR Line 38: declare JPACKAGE_MIRROR Line 39: declare COPR I'd put the defaults here, as this is the same as: var="$var" informational only, if you add the defaults here it becomes more informational and simplifies the parsing code Line 40: Line 41: #logfile="running.log" Line 42: #touch $logfile Line 43: #hr="=============================================================================" Line 48: { Line 49: local item Line 50: for item in "${MY_TMP_TRASH[@]}" Line 51: do Line 52: if [[ ${TEST_RUN} -ne 0 ]]; then what if it's sourced? Line 53: ## on test run we don't want to cleanup automatically Line 54: break Line 55: else Line 56: echo "rm_on_exit running: rm -fr ${item}" Line 60: } Line 61: Line 62: add_rm_on_exit() Line 63: { Line 64: local idx=${#MY_TMP_TRASH[*]} use @ instead of * Line 65: MY_TMP_TRASH[$idx]="$*" Line 66: if [[ $idx -eq 0 ]]; then Line 67: echo "Setting trap" Line 68: trap rm_on_exit EXIT Line 61: Line 62: add_rm_on_exit() Line 63: { Line 64: local idx=${#MY_TMP_TRASH[*]} Line 65: MY_TMP_TRASH[$idx]="$*" you can use: MY_TMP_TRASH+=("$@") that will handle well spaces and avoid the index extra logic Line 66: if [[ $idx -eq 0 ]]; then Line 67: echo "Setting trap" Line 68: trap rm_on_exit EXIT Line 69: fi Line 62: add_rm_on_exit() Line 63: { Line 64: local idx=${#MY_TMP_TRASH[*]} Line 65: MY_TMP_TRASH[$idx]="$*" Line 66: if [[ $idx -eq 0 ]]; then no need for this if, trap will replace the existing trap if it already exists Line 67: echo "Setting trap" Line 68: trap rm_on_exit EXIT Line 69: fi Line 70: } Line 129: esac Line 130: done Line 131: } Line 132: Line 133: set_if_undefined() { use var="${var:-default}" Line 134: # sets variable "name" with "value" if it is undef (not set) Line 135: local name="${1}" Line 136: local value="${@:2}" Line 137: if [[ -z "$( eval "\${${name}+x}" )" ]]; then Line 161: for distro_layout_map in "${DISTRO_LAYOUTS[@]}"; do Line 162: oldIFS="$IFS" Line 163: IFS=: Line 164: read distro layout <<< "${distro_layout_map}" Line 165: IFS=$oldIFS use internal tools: d="${lm%%:*}" l="${lm#*:}" Line 166: Line 167: [[ "${distro}" == "${DISTRIBUTION}" ]] || continue Line 168: message="Distribution: ${distro} uses layout: ${layout}" Line 169: [[ "${layout}" == "${LAYOUT}" ]] || die 1 "${message}" Line 520: "${look_aside}" Line 521: "--repoid=${check_repo}" Line 522: ) Line 523: #echo cmd: "${cmd[@]}" Line 524: ${cmd[@]} || die $? "${msg}" quote to avoid space issues: "${cmd[@]}" Line 525: ## cleanup our mess if it all went well: Line 526: if [[ -d "${yumconfdir}" ]]; then Line 527: rm -fr "${yumconfdir}" Line 528: fi Line 536: # check_repo_closure Line 537: check_repo_closure_new "${DISTRIBUTION}" "${DISTRIBUTION_VERSION}" Line 538: } Line 539: Line 540: if [[ "${0}" == *repo_closure_check.sh ]]; then consider using =~ as it does not have issues with spaces and it's a lot clearer it's a regexp Line 541: # only runs when we're not sourced is 0. Line 542: main "${@}" Line 543: exit $? -- To view, visit https://gerrit.ovirt.org/39249 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I207c5c59ca3149c17897c231e391e15e7eec4363 Gerrit-PatchSet: 22 Gerrit-Project: jenkins Gerrit-Branch: master Gerrit-Owner: Max Kovgan <m...@redhat.com> Gerrit-Reviewer: David Caro <dcaro...@redhat.com> Gerrit-Reviewer: Max Kovgan <m...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@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
