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.

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

IntelliJ: automated bash fixes #1777

Merged
merged 1 commit into from Jun 29, 2018
Merged

IntelliJ: automated bash fixes #1777

merged 1 commit into from Jun 29, 2018

Conversation

dhalperi
Copy link
Member

  • simple variables ($foo to ${foo})
  • missing shebang lines
  • path fixups when bash scripts reference other scripts

@dhalperi dhalperi requested a review from arifogel June 28, 2018 06:58
@batfish-bot
Copy link

This change is Reviewable

@codecov
Copy link

codecov bot commented Jun 28, 2018

Codecov Report

Merging #1777 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #1777      +/-   ##
============================================
- Coverage     67.83%   67.83%   -0.01%     
- Complexity    15601    15602       +1     
============================================
  Files          1427     1427              
  Lines         74280    74280              
  Branches       9788     9788              
============================================
- Hits          50389    50387       -2     
- Misses        20150    20155       +5     
+ Partials       3741     3738       -3

@arifogel
Copy link
Member

Reviewed 14 of 14 files at r1.
Review status: all files reviewed, 8 unresolved discussions (waiting on @dhalperi)


a discussion (no related file):
Feel free to ignore any/all requests for adding quotes. I made none of them blocking.


.travis/install.sh, line 10 at r1 (raw file):

   z3 --version || exit 1
elif [[ ${TRAVIS_OS_NAME} == 'osx' ]]; then
   sudo ${TRAVIS_BUILD_DIR}/tools/install_z3_osx.sh || exit 1

quote path


projects/allinone/allinone, line 3 at r1 (raw file):

#!/bin/bash
ALLINONE="${BASH_SOURCE[0]}"
ALLINONE_PATH=$(dirname "$ALLINONE")

quote outer


projects/batfish-client/batfish-client, line 2 at r1 (raw file):

#!/bin/bash
BATFISH_CLIENT=${BASH_SOURCE[0]}

quote


projects/batfish-client/batfish-client, line 3 at r1 (raw file):

#!/bin/bash
BATFISH_CLIENT=${BASH_SOURCE[0]}
BATFISH_CLIENT_PATH=$(dirname ${BATFISH_CLIENT})

quote inner and outer


tools/common.sh, line 142 at r1 (raw file):

   # call with a prompt string or use a default
   read -r -p "${1:-Are you sure? [y/N]} " response < /dev/tty
   case ${response} in

quote since arbitrary text?


tools/common.sh, line 190 at r1 (raw file):

   cp -r ${BATFISH_CLIENT_PATH}/doc ${BATFISH_ROOT}/doc/batfish-client/
   cp -r ${COORDINATOR_PATH}/doc ${BATFISH_ROOT}/doc/coordinator/
   cp -r ${ALLINONE_PATH}/doc ${BATFISH_ROOT}/doc/allinone/

quote all these paths. Seems to apply nearly everywhere you added braces in rest of file.


tools/package_rpm.sh, line 82 at r1 (raw file):

   set -e
   if [ "${REDHAT_VERSION}" = "7" ]; then
      cat > ${BATFISH_INIT_P} <<EOF

quote every var or path containing var not ending in _ARGS


tools/package_ubuntu.sh, line 61 at r1 (raw file):

   set -e
   if [ "${UBUNTU_VERSION}" = "16.04" ]; then
      cat > ${BATFISH_INIT_P} <<EOF

quote every var or path containing var not ending in _ARGS


Comments from Reviewable

@arifogel
Copy link
Member

:lgtm: modulo optional addition of quotes


Review status: all files reviewed, 8 unresolved discussions (waiting on @dhalperi)


Comments from Reviewable

@dhalperi dhalperi merged commit 41436fd into master Jun 29, 2018
@dhalperi dhalperi deleted the intellij-bash branch June 29, 2018 04:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants