From 1ea7fbb21ca889d124f2c5b210999e2a13588117 Mon Sep 17 00:00:00 2001 From: Imran Iqbal Date: Wed, 27 Nov 2019 13:33:42 +0000 Subject: [PATCH 1/3] feat(shellcheck): apply fixes throughout this repo --- pre-commit_semantic-release.sh | 6 ++--- ssf/files/default/git/git_10_prepare.sh | 18 ++++++++------- ssf/files/default/git/git_20_commit_push.sh | 22 +++++++++++-------- ssf/files/default/git/git_30_create_PR.sh | 10 ++++----- .../default/pre-commit_semantic-release.sh | 6 ++--- 5 files changed, 34 insertions(+), 28 deletions(-) diff --git a/pre-commit_semantic-release.sh b/pre-commit_semantic-release.sh index 9d34d74c..ba805352 100755 --- a/pre-commit_semantic-release.sh +++ b/pre-commit_semantic-release.sh @@ -14,9 +14,9 @@ sed -i -e "s_^\(version:\).*_\1 ${1}_" FORMULA sudo -H pip install m2r # Copy and then convert the `.md` docs -cp *.md docs/ -cd docs/ -m2r --overwrite *.md +cp ./*.md docs/ +cd docs/ || exit +m2r --overwrite ./*.md # Change excess `H1` headings to `H2` in converted `CHANGELOG.rst` sed -i -e '/^=.*$/s/=/-/g' CHANGELOG.rst diff --git a/ssf/files/default/git/git_10_prepare.sh b/ssf/files/default/git/git_10_prepare.sh index 032d5378..52b1794d 100755 --- a/ssf/files/default/git/git_10_prepare.sh +++ b/ssf/files/default/git/git_10_prepare.sh @@ -14,23 +14,23 @@ COMMENT='Command `'${STATE}'` run' # Check if PR branch already exists BRANCH=$(git branch -l "${BRANCH_PR}") COMMIT= -if [ ! -z "${BRANCH}" ]; then - git checkout ${BRANCH_PR} +if [ -n "${BRANCH}" ]; then + git checkout "${BRANCH_PR}" # This may end up as blank as well, so that's why a separate `if` is required below COMMIT=$(git log -n1 | grep "${COMMIT_GREP}") fi # Perform actions depending on if a commit was found or not -if [ ! -z "${COMMIT}" ]; then +if [ -n "${COMMIT}" ]; then CHANGED='False' else - git checkout ${BRANCH_UPSTREAM} + git checkout "${BRANCH_UPSTREAM}" git pull git status # If the branch existed but not the commit, assume the branch is stale (i.e. previous PR merged) # Remove it, ready to be recreated at the latest upstream commit - if [ ! -z "${BRANCH}" ]; then - git branch -d ${BRANCH_PR} + if [ -n "${BRANCH}" ]; then + git branch -d "${BRANCH_PR}" fi # TODO: Improve this part, should be able to remove with the duplication with the right solution # Don't want to resort to using `git branch -D` above, since that could be premature in certain situations @@ -40,8 +40,10 @@ else if [ -z "${BRANCH}" ]; then NEW_BRANCH='-b' fi - git checkout ${NEW_BRANCH} ${BRANCH_PR} - git merge ${BRANCH_UPSTREAM} + # Disabling `SC2086` where double-quoting an empty variable introduces an error + # shellcheck disable=SC2086 + git checkout ${NEW_BRANCH} "${BRANCH_PR}" + git merge "${BRANCH_UPSTREAM}" fi # Write the state line diff --git a/ssf/files/default/git/git_20_commit_push.sh b/ssf/files/default/git/git_20_commit_push.sh index ec1f4aaa..5f6fa3d2 100755 --- a/ssf/files/default/git/git_20_commit_push.sh +++ b/ssf/files/default/git/git_20_commit_push.sh @@ -10,10 +10,11 @@ COMMIT_GREP=${4} COMMIT_TITLE=${5} COMMIT_BODY=${6} COMMIT_OPTIONS=${7} -PUSH_ACTIVE=$(echo ${8} | tr "[:upper:]" "[:lower:]") -PUSH_VIA_PR=$(echo ${9} | tr "[:upper:]" "[:lower:]") +PUSH_ACTIVE=$(echo "${8}" | tr "[:upper:]" "[:lower:]") +PUSH_VIA_PR=$(echo "${9}" | tr "[:upper:]" "[:lower:]") REMOTE_FORK_NAME=${10} -REMOTE_FORK_BRANCH=${11} +# Currently unused but leaving here as a placeholder +# REMOTE_FORK_BRANCH=${11} REMOTE_UPSTREAM_NAME=${12} REMOTE_UPSTREAM_BRANCH=${13} # Prepare initial state line variables @@ -22,7 +23,7 @@ COMMENT='Command `'${STATE}'` run' # Prepare git options depending on if a commit was found or not COMMIT=$(git log -n1 | grep "${COMMIT_GREP}") -if [ ! -z "${COMMIT}" ]; then +if [ -n "${COMMIT}" ]; then AMEND='--amend' FORCE='-f' else @@ -31,15 +32,18 @@ else fi # Perform actions +# Disabling `SC2086` where double-quoting an empty variable introduces errors +# shellcheck disable=SC2086 git commit ${AMEND} "${COMMIT_OPTIONS}" -m "${COMMIT_TITLE}" -m "${COMMIT_BODY}" if ${PUSH_ACTIVE}; then if ${PUSH_VIA_PR}; then - git push ${FORCE} -u ${REMOTE_FORK_NAME} ${BRANCH_PR} + # shellcheck disable=SC2086 + git push ${FORCE} -u "${REMOTE_FORK_NAME}" "${BRANCH_PR}" else - git checkout ${BRANCH_UPSTREAM} - git merge ${BRANCH_PR} - git push ${REMOTE_UPSTREAM_NAME} HEAD:${REMOTE_UPSTREAM_BRANCH} - git branch -d ${BRANCH_PR} + git checkout "${BRANCH_UPSTREAM}" + git merge "${BRANCH_PR}" + git push "${REMOTE_UPSTREAM_NAME}" "HEAD:${REMOTE_UPSTREAM_BRANCH}" + git branch -d "${BRANCH_PR}" fi fi diff --git a/ssf/files/default/git/git_30_create_PR.sh b/ssf/files/default/git/git_30_create_PR.sh index e0c63e60..8e255ef3 100755 --- a/ssf/files/default/git/git_30_create_PR.sh +++ b/ssf/files/default/git/git_30_create_PR.sh @@ -22,18 +22,18 @@ COMMENT='Command `'${STATE}'` run' # Only create the PR if it doesn't already exist # If it already exists, the `git push` done earlier will have updated the PR already -PR_EXISTS=$(curl -i https://api.github.com/repos/${REPO_OWNER}/${REPO_NAME}/pulls | grep "${GH_USER}:${BRANCH_PR}") -if [ ! -z "${PR_EXISTS}" ]; then +PR_EXISTS=$(curl -i "https://api.github.com/repos/${REPO_OWNER}/${REPO_NAME}/pulls" | grep "${GH_USER}:${BRANCH_PR}") +if [ -n "${PR_EXISTS}" ]; then CHANGED='False' else curl -H "Authorization: bearer ${GH_TOKEN}" -d ' { "title": "'"${COMMIT_TITLE}"'", "body": "'"${COMMIT_BODY}"'", - "head": "'${GH_USER}':'${BRANCH_PR}'", - "base": "'${REPO_BRANCH}'" + "head": "'"${GH_USER}"':'"${BRANCH_PR}"'", + "base": "'"${REPO_BRANCH}"'" } - ' https://api.github.com/repos/${REPO_OWNER}/${REPO_NAME}/pulls >> ${FILE_API_RESPONSE} + ' "https://api.github.com/repos/${REPO_OWNER}/${REPO_NAME}/pulls" >> "${FILE_API_RESPONSE}" fi # Write the state line diff --git a/ssf/files/default/pre-commit_semantic-release.sh b/ssf/files/default/pre-commit_semantic-release.sh index 9d34d74c..ba805352 100755 --- a/ssf/files/default/pre-commit_semantic-release.sh +++ b/ssf/files/default/pre-commit_semantic-release.sh @@ -14,9 +14,9 @@ sed -i -e "s_^\(version:\).*_\1 ${1}_" FORMULA sudo -H pip install m2r # Copy and then convert the `.md` docs -cp *.md docs/ -cd docs/ -m2r --overwrite *.md +cp ./*.md docs/ +cd docs/ || exit +m2r --overwrite ./*.md # Change excess `H1` headings to `H2` in converted `CHANGELOG.rst` sed -i -e '/^=.*$/s/=/-/g' CHANGELOG.rst From f52eb378987ac0cacaf3a079ca03067107173661 Mon Sep 17 00:00:00 2001 From: Imran Iqbal Date: Wed, 27 Nov 2019 13:34:37 +0000 Subject: [PATCH 2/3] feat(travis): run `shellcheck` during lint job * https://github.com/saltstack-formulas/template-formula/pull/180#issuecomment-558612422 --- .travis.yml | 6 +++++- ssf/defaults.yaml | 4 ++-- ssf/files/default/.travis.yml | 8 ++++++-- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/.travis.yml b/.travis.yml index 6eaad65f..eac5d509 100644 --- a/.travis.yml +++ b/.travis.yml @@ -24,7 +24,7 @@ jobs: - language: 'node_js' node_js: 'lts/*' env: 'Lint' - name: 'Lint: salt-lint, yamllint, rubocop & commitlint' + name: 'Lint: salt-lint, yamllint, rubocop, shellcheck & commitlint' before_install: 'skip' script: # Install and run `salt-lint` @@ -38,6 +38,10 @@ jobs: # Install and run `rubocop` - gem install rubocop - rubocop -d + # Run `shellcheck` (already pre-installed in Travis) + - shellcheck --version + - git ls-files | grep '\.sh$\|\.bash$\|\.ksh$' + | xargs shellcheck # Install and run `commitlint` - npm i -D @commitlint/config-conventional @commitlint/travis-cli diff --git a/ssf/defaults.yaml b/ssf/defaults.yaml index 48e7fb48..21a21798 100644 --- a/ssf/defaults.yaml +++ b/ssf/defaults.yaml @@ -22,8 +22,8 @@ ssf_node_anchors: # An alternative method could be to use: # `git describe --abbrev=0 --tags` # yamllint disable rule:line-length - title: 'ci(travis): use default matrix after `centos-6` image fix' - body: '* Automated using https://github.com/myii/ssf-formula/pull/105' + title: 'ci(travis): run `shellcheck` during lint job [skip ci]' + body: '* Automated using https://github.com/myii/ssf-formula/pull/106' # yamllint enable rule:line-length github: owner: 'saltstack-formulas' diff --git a/ssf/files/default/.travis.yml b/ssf/files/default/.travis.yml index 0b9fa9e1..5ef15789 100644 --- a/ssf/files/default/.travis.yml +++ b/ssf/files/default/.travis.yml @@ -23,10 +23,10 @@ fast_finish: true {#- Prepare variables used for linters #} {%- set comment_linters = '# Run all of the linters in a single job' %} -{%- set name_linters = 'Lint: salt-lint, yamllint, rubocop & commitlint' %} +{%- set name_linters = 'Lint: salt-lint, yamllint, rubocop, shellcheck & commitlint' %} {%- if not travis.use_single_job_for_linters %} {%- set comment_linters = comment_linters ~ ' (except `rubocop`)' %} -{%- set name_linters = 'Lint: salt-lint, yamllint & commitlint' %} +{%- set name_linters = 'Lint: salt-lint, yamllint, shellcheck & commitlint' %} {%- endif %} {#- Prepare variable used for `saltcheck` #} {%- set use_saltcheck = False %} @@ -137,6 +137,10 @@ jobs: {%- if travis.use_single_job_for_linters %} {{- format_rubocop_linter() }} {%- endif %} + # Run `shellcheck` (already pre-installed in Travis) + - shellcheck --version + - git ls-files | grep '\.sh$\|\.bash$\|\.ksh$' + | xargs shellcheck # Install and run `commitlint` - npm i -D @commitlint/config-conventional @commitlint/travis-cli From 615e3b2e598d8e91bca5ba8d681778af61429c9e Mon Sep 17 00:00:00 2001 From: Imran Iqbal Date: Wed, 27 Nov 2019 19:53:21 +0000 Subject: [PATCH 3/3] refactor(travis): use pathspecs for `git ls-files` instead of `grep` * Apply suggestion received here: - https://github.com/saltstack-formulas/template-formula/pull/181#discussion_r351421463 --- .travis.yml | 8 ++++---- ssf/files/default/.travis.yml | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/.travis.yml b/.travis.yml index eac5d509..f59383e2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -29,8 +29,8 @@ jobs: script: # Install and run `salt-lint` - pip install --user salt-lint - - git ls-files | grep '\.sls$\|\.jinja$\|\.j2$\|\.tmpl$\|\.tst$' - | xargs salt-lint + - git ls-files -- *.sls *.jinja *.j2 *.tmpl *.tst + | xargs salt-lint # Install and run `yamllint` # Need at least `v1.17.0` for the `yaml-files` setting - pip install --user yamllint>=1.17.0 @@ -40,8 +40,8 @@ jobs: - rubocop -d # Run `shellcheck` (already pre-installed in Travis) - shellcheck --version - - git ls-files | grep '\.sh$\|\.bash$\|\.ksh$' - | xargs shellcheck + - git ls-files -- *.sh *.bash *.ksh + | xargs shellcheck # Install and run `commitlint` - npm i -D @commitlint/config-conventional @commitlint/travis-cli diff --git a/ssf/files/default/.travis.yml b/ssf/files/default/.travis.yml index 5ef15789..7b1b3720 100644 --- a/ssf/files/default/.travis.yml +++ b/ssf/files/default/.travis.yml @@ -128,8 +128,8 @@ jobs: {%- endif %} # Install and run `salt-lint` - {{ pip_cmd }} install --user salt-lint - - git ls-files | grep '\.sls$\|\.jinja$\|\.j2$\|\.tmpl$\|\.tst$' - | xargs salt-lint + - git ls-files -- *.sls *.jinja *.j2 *.tmpl *.tst + | xargs salt-lint # Install and run `yamllint` # Need at least `v1.17.0` for the `yaml-files` setting - {{ pip_cmd }} install --user yamllint>=1.17.0 @@ -139,8 +139,8 @@ jobs: {%- endif %} # Run `shellcheck` (already pre-installed in Travis) - shellcheck --version - - git ls-files | grep '\.sh$\|\.bash$\|\.ksh$' - | xargs shellcheck + - git ls-files -- *.sh *.bash *.ksh + | xargs shellcheck # Install and run `commitlint` - npm i -D @commitlint/config-conventional @commitlint/travis-cli