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

[dev] Remove shellcheck from make checkstatic #6842

Merged
merged 1 commit into from Jul 16, 2020

Conversation

driusan
Copy link
Collaborator

@driusan driusan commented Jul 15, 2020

PR #6053 added the "shellcheck" tool to our static analysis suite
to lint shell scripts. However, the PR added it to make checkstatic
prematurely.

  1. The wrapper shell script does not run on MacOS because it depends
    on newer bash options.
  2. The script requires a third party tool which is not managed by
    any of our dependency managers (npm, composer, etc) to be installed.

The result is that developers can no longer run make checkstatic
to check their code without running through hoops. This removes
it from the checkstatic target, since it currently prevents developers
from working on LORIS.

PR aces#6053 added the "shellcheck" tool to our static analysis suite
to lint shell scripts. However, the PR added it to `make checkstatic`
prematurely.

1. The wrapper shell script does not run on MacOS because it depends
   on newer bash options.
2. The script requires a third party tool which is not managed by
   any of our dependency managers (npm, composer, etc) to be installed.

The result is that developers can no longer run `make checkstatic`
to check their code without running through hoops. This removes
it from the checkstatic target, since it currently prevents developers
from working on LORIS.
@driusan driusan closed this Jul 16, 2020
@driusan driusan reopened this Jul 16, 2020
@driusan driusan merged commit ac4222d into aces:master Jul 16, 2020
@ridz1208 ridz1208 added this to the 24.0.0 milestone Jul 27, 2020
AlexandraLivadas pushed a commit to AlexandraLivadas/Loris that referenced this pull request Jun 29, 2021
PR aces#6053 added the "shellcheck" tool to our static analysis suite
to lint shell scripts. However, the PR added it to `make checkstatic`
prematurely.

1. The wrapper shell script does not run on MacOS because it depends
   on newer bash options.
2. The script requires a third party tool which is not managed by
   any of our dependency managers (npm, composer, etc) to be installed.

The result is that developers can no longer run `make checkstatic`
to check their code without running through hoops. This removes
it from the checkstatic target, since it currently prevents developers
from working on LORIS.
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

2 participants