Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

Passing options to shellcheck #12

Merged
merged 4 commits into from
Aug 21, 2018
Merged

Passing options to shellcheck #12

merged 4 commits into from
Aug 21, 2018

Conversation

durkode
Copy link
Contributor

@durkode durkode commented Jul 19, 2018

This PR enables options to be passed to the shellcheck docker image, usage as such:

steps:
  - plugins:
      shellcheck#v1.0.1:
        files: scripts/*.sh
        options:
          - --exclude=SC2207,SC2154
          - --format=gcc

When making a release from this, please update the Readme to reflect this.

@durkode durkode mentioned this pull request Jul 19, 2018
Closed
@joscha
Copy link

joscha commented Aug 21, 2018

@lox can we get this in, please?

lox
lox reviewed Aug 21, 2018
View reviewed changes
hooks/command
@@ -34,8 +34,11 @@ if [[ -z ${files:-} ]] ; then
exit 1
fi

# Read in the options to pass to shellcheck
mapfile -t options<<<"$(plugin_read_list OPTIONS)"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm mildly concerned that this pattern comes with a bash 4.x+ requirement.

Copy link
Contributor

@toolmantim toolmantim Aug 21, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

If I'm not mistaken, does this mean I can't run the shellcheck plugin anymore via bksr on my Mac? (bash 3.2)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I didn't realise mapfile was 4.x+. This has been merged already but if this is an issue you should be able to change it back to options=( $(plugin_read_list OPTIONS) ), which ironically wouldn't pass SC2207.

Copy link
Contributor

Choose a reason for hiding this comment

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

I wish MacOS wasn’t so behind the times 😢

Copy link
Contributor

Choose a reason for hiding this comment

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

Fixed in #15.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also updated the Shellcheck wiki 😅https://github.com/koalaman/shellcheck/wiki/SC2207

Copy link
Contributor

Choose a reason for hiding this comment

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

🙏🏼

@lox
Copy link
Contributor

lox commented Aug 21, 2018

Thanks for the ping @joscha. Let's just ship this.

@lox lox merged commit e560f13 into buildkite-plugins:master Aug 21, 2018
@lox
Copy link
Contributor

lox commented Aug 21, 2018

Behold: https://github.com/buildkite-plugins/shellcheck-buildkite-plugin/releases/tag/v1.1.0

Thanks very much @durkode, sorry it took us so long. 🙇🏼‍♂️

# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@lox lox lox left review comments

@toolmantim toolmantim toolmantim left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@durkode @joscha @lox @toolmantim