Skip to content

Conversation

Kreyren
Copy link

@Kreyren Kreyren commented Dec 1, 2019

I'm forking this configuration in https://github.com/Kreyrock/Kreyrock so i rewrote it to what i consider better code quality excluding over-sanitization in this scenario.

  • Now this is compatible with POSIX shell and bash
  • Fixed Shellcheck excluding SC1090 since i don't know where the source for it is (probably in docker image to which we don't have an access?)
  • Unset variables since shell doesn't support local var=value
  • Added simplified error handling
  • Replaced echo with printf which is sligtly faster (https://unix.stackexchange.com/a/77564) and more reliable POSIX and cross-platform (https://unix.stackexchange.com/a/65819)
  • Rewrote if statements to be more readable
  • Replaced ${var} with $var since they are not needed and still require double quoting

Signed-off-by: Jacob Hrbek [email protected]

@Kreyren
Copy link
Author

Kreyren commented Dec 1, 2019

Current shellcheck (http://ix.io/23fG) compared to proposed (http://ix.io/23fH)

I'm forking this configuration in https://github.com/Kreyrock/Kreyrock 
so i rewrote it to what i consider better code quality excluding 
over-sanitization in this scenario.
- Now this is compatible with POSIX shell and bash
- Fixed Shellcheck excluding SC1090 since i don't know where the source 
for it is (probably in docker image to which we don't have an access?)
- Unset variables since shell doesn't support `local var=value`
- Added simplified error handling
- Replaced echo with printf which is sligtly faster 
(https://unix.stackexchange.com/a/77564) and more reliable POSIX and 
cross-platform (https://unix.stackexchange.com/a/65819)
- Rewrote if statements to be more readable
- Replaced ${var} with $var since they are not needed and still require 
double quoting

Signed-off-by: Jacob Hrbek <[email protected]>
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.

1 participant