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

De-bash: remove function keyword from shell scripts #227

Closed
wants to merge 1 commit into from
Closed

De-bash: remove function keyword from shell scripts #227

wants to merge 1 commit into from

Conversation

rawiriblundell
Copy link
Contributor

The 'function' keyword is a non-portable bashism that is considered deprecated and obsolete. This PR simply removes it from all identified shell scripts.

@jherbel
Copy link
Contributor

jherbel commented Aug 10, 2020

Hello Rawiri,

thank you for this pull request. We will have a closer look at your patch and discuss internally on how and if we can implement it into the official code base.

Internal Ref: CMK-5166

@rawiriblundell
Copy link
Contributor Author

Hi,
this is a fairly straightforward change and I'm not sure why it's been sitting here so long. Extra supporting documentation:

https://wiki.bash-hackers.org/scripting/obsolete
https://wiki.ubuntu.com/DashAsBinSh#function
https://github.com/tribe29/checkmk/blob/master/CONTRIBUTING.md#function-names

The function keyword will also throw an error in shellcheck if run with a more POSIX strict shell like dash.

Can we please get this merged?

Cheers

@svenpanne
Copy link
Contributor

The PR looks sensible to me, could you please upload a squashed version rebased on master? (i.e. a single commit on top of the current master) I have trouble rebasing it...

@rawiriblundell
Copy link
Contributor Author

pylint failure (yet again :) ) aside, this has been done.

@svenpanne
Copy link
Contributor

OK, I've fixed the CI failures which we had for some weeks now:

  • one missing pylint suppression needed for the raw edition only
  • skip tests which rely on OpenSSL 1.1, Travis still runs Xenial

After that, I could cleanly merge your PR. Thanks!

@svenpanne svenpanne closed this Jan 7, 2021
@rawiriblundell rawiriblundell deleted the debash_function_keyword branch January 16, 2021 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants