charlesdaniels / bitshuffle

BSD 3-Clause "New" or "Revised" License
5 stars 0 forks source link

Added shellcheck, misc. CI improvements #41

Closed charlesdaniels closed 6 years ago

charlesdaniels commented 6 years ago
charlesdaniels commented 6 years ago

Hmm, it yelled at me for using local, because it was not POSIX compliant. There is a shellcheck flag for this though.

I used the wrapper script because Ubuntu 14 does not package shellcheck (?), and even the version on 16 is too old to be useful. I have heard of some projects pulling down Debian 9’s version, so that might be worth a shot.


This email was sent from my smartphone.

On Dec 19, 2017, at 12:30, Joshua Nelson notifications@github.com wrote:

@jyn514 requested changes on this pull request.

I would rather not used the specific wrapper you chose for shellcheck. I do approve of the idea, but I think downloading the apt package and writing our own wrapper would be the way to go (instead of installing ghc). Also note that all of the checks should enforce posix compliance, which they do not currently do.

In .travis.yml:

@@ -2,20 +2,39 @@ language: python python:

  • "2.7"
  • "3.6"
    • "nightly"
  • I approve. Do we want to support python 2.6? I vote no, it's not used much and it'll take more effort than its worth. If so, however, now would be the time.

In .travis.yml:

Sudo is required so that steup.py can be tested (TODO: investigate using

a virtualenv instead?)

sudo: required +addons:

  • apt:
  • packages:
    • realpath # required by script
    • cabal-install
    • ghc What's going on here? As far as I'm aware, shellcheck is a binary that can be installed with sudo apt install shellcheck.

In .travis.yml:

os: linux install:

  • pip install pycodestyle setuptools
  • gem install travis --no-rdoc --no-ri

In scripts/realpath.sh:

@@ -131,7 +128,7 @@ _emulated_readlink() { }

_gnu_stat_readlink() {

  • local output
  • output Useless declaration

In .travis.yml:

os: linux install:

  • pip install pycodestyle setuptools
  • gem install travis --no-rdoc --no-ri

In scripts/pypi.sh:

@@ -2,6 +2,6 @@ ./setup.py sdist ./setup.py bdist_wheel printf 'Upload project to PyPI? [no]: ' -read confirm +read -r confirm if [[ $confirm = y ]] ; then twine upload dist/; fi Not posix compliant (and not caught by the shellcheck used)

In scripts/pre_commit_check.sh:

@@ -8,8 +8,9 @@ #

.ENDOC

-. "$(dirname $0)/realpath.sh" -PARENT_DIR="$(realpath_sh $(dirname "$0"))" +# shellcheck disable=SC1090 +. "$(dirname "$0")/realpath.sh" Is this portable? Does it work in pure sh implementations?

In scripts/test_shellcheck.sh:

+set -u + +# shellcheck disable=SC1090 +. "$(dirname "$0")/realpath.sh" +PARENT_DIR="$(realpath_sh "$(dirname "$0")")" +PROJECT_ROOT="$PARENT_DIR/.." + +TOTAL=0 +RETCODE=0 +TEMPFILE="/tmp/$(uuidgen)" +# the grep -v '.git' prevents git's default hooks from causing ShellCheck +# defects. +find "$PROJECT_ROOT" -print | grep -v '.git' | while read -r line ; do

  • if file "$line" | grep -i 'shell script' > /dev/null 2>&1 ; then
  • echo "shellcheck for file '$line': "
  • shellcheck "$line" shellcheck -s sh "$line"

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

jyn514 commented 6 years ago

Going to attempt using travis: https://github.com/koalaman/shellcheck#travis-ci-setup