Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixing shellcheck warnings in prin #763

Merged
merged 1 commit into from
Jun 30, 2019
Merged

Fixing shellcheck warnings in prin #763

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 13 additions & 11 deletions prin
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,10 @@ PROG=${0##*/}
# If NO ARGUMENTS should return *usage*, uncomment the following line:
usage=${1:-yes}

source $(dirname $(readlink -ne $BASH_SOURCE))/lib/common.sh
source $TOOL_LIB_PATH/gitlib.sh
# shellcheck source=./lib/common.sh
source "$(dirname "$(readlink -ne ${BASH_SOURCE[0]})")/lib/common.sh"
# shellcheck source=./lib/gitlib.sh
source "$TOOL_LIB_PATH/gitlib.sh"

###############################################################################
# FUNCTIONS
Expand All @@ -70,7 +72,7 @@ show_tags () {
local pr=$1
local initial_commit=$2
shift 2
local cp_commits=($*)
local cp_commits=("$*")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a good chance this should be "$@" instead, ref: https://github.com/koalaman/shellcheck/wiki/SC2086#exceptions

local commit
local commit_seconds
local tag_seconds
Expand All @@ -97,11 +99,11 @@ show_tags () {
fi

# We only care about the initial commit for latency calculation
commit_seconds=$(git show -s --format="%ct" $initial_commit)
commit_seconds=$(git show -s --format="%ct" "$initial_commit")

for commit in $initial_commit ${cp_commits[*]}; do
for tag in $(git tag --contains $commit); do
tag_seconds=$(git show -s --pretty=format:%ct $tag |tail -1)
for tag in $(git tag --contains "$commit"); do
tag_seconds=$(git show -s --pretty=format:%ct "$tag" |tail -1)

# Convert to days with some precision
days=$(echo "scale=2; ($tag_seconds-$commit_seconds)/86400"|bc)
Expand All @@ -121,7 +123,7 @@ show_tags () {
###############################################################################
# Stubbed out
security_layer::hosted_map () {
logecho "Skipping $FUNCNAME extensions..."
logecho "Skipping ${FUNCNAME[0]} extensions..."
return 0
}

Expand All @@ -142,15 +144,15 @@ if [[ ${POSITIONAL_ARGV[0]} =~ [0-9]{5,} ]]; then
# non-cherrypick commit searches on branches
INITIAL_COMMIT=$(git log --grep "Merge pull request #$PR" \
--all --pretty=format:"%h")
CP_COMMITS=($(git log --grep "cherry-pick-of-.*#$PR-" \
--all --pretty=format:"%h"))
mapfile -i CP_COMMITS < <(git log --grep "cherry-pick-of-.*#$PR-" \
--all --pretty=format:"%h")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pswica, here I met this error when used mapfile -i :
/Users/llhu/Desktop/test/release/release/prin: line 154: mapfile: -i: invalid option mapfile: usage: mapfile [-d delim] [-n count] [-O origin] [-s count] [-t] [-u fd] [-C callback] [-c quantum] [array]
Is there some errors in my local side. Thanks

Copy link
Contributor Author

@pswica pswica Jul 1, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much for catching. No this was my bad. I messed up by putting -i instead of -t

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR here

else
# TODO: Need to validate this is a git hash
INITIAL_COMMIT=("${POSITIONAL_ARGV[0]}")
fi

[[ -z "$INITIAL_COMMIT" ]] && common::exit 1 "No commit/pr found in this repo."
[[ -z "${INITIAL_COMMIT[0]}" ]] && common::exit 1 "No commit/pr found in this repo."

security_layer::hosted_map

show_tags "$PR" $INITIAL_COMMIT ${CP_COMMITS[*]}
show_tags "$PR" "${INITIAL_COMMIT[0]}" "${CP_COMMITS[*]}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a good chance this should be "${CP_COMMITS[@]}" instead, ref: https://github.com/koalaman/shellcheck/wiki/SC2086#exceptions

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for reading these. I'm sure there is a lot of hidden bugs/weirdness from my shellcheck changes. I will create an issue and link this, and other things found, to it