Skip to content

Improve coding style of shell scripts #742

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 18, 2018
Merged

Improve coding style of shell scripts #742

merged 1 commit into from
May 18, 2018

Conversation

@PeterDaveHello
Copy link
Member

@PeterDaveHello PeterDaveHello commented May 17, 2018

Working on readability and maintainability.

@PeterDaveHello PeterDaveHello requested a review from a team May 17, 2018 16:36
@PeterDaveHello PeterDaveHello mentioned this pull request May 17, 2018
Merged
chorrell

This comment was marked as off-topic.

Sign in to view

@PeterDaveHello PeterDaveHello force-pushed the improve-coding-style branch from 6639c4e to b04415f Compare May 17, 2018 18:35
@PeterDaveHello
Copy link
Member Author

PeterDaveHello commented May 17, 2018

As we'll use Linux to build the images, maybe #!/bin/bash would be good enough, I changed to use env though.

@chorrell
Copy link
Contributor

chorrell commented May 17, 2018

Yeah, either is good as long as it’s consistent

@PeterDaveHello
Copy link
Member Author

PeterDaveHello commented May 17, 2018

Yeah, and if we could rewrite to pure POSIX compatible version, then we could simply use #!/bin/sh in the future! Will try to do then when I have free time! Any other suggestions would be welcome.

@PeterDaveHello PeterDaveHello force-pushed the improve-coding-style branch from b04415f to bb92568 Compare May 17, 2018 19:03
@LaurentGoderre
Copy link
Member

LaurentGoderre commented May 17, 2018

Doesn't it work in POSIX. I do a lt of this on my Mac which is POSIX

@PeterDaveHello
Copy link
Member Author

PeterDaveHello commented May 18, 2018

[[ ]] is bash syntax, not POSIX, if I didn't remember it wrong, anyway, unify the style would be good ;)

LaurentGoderre

This comment was marked as off-topic.

Sign in to view

@LaurentGoderre
Copy link
Member

LaurentGoderre commented May 18, 2018

Can we merge this sooner or later since it affects all other PR that modify the shell scripts

chorrell

This comment was marked as off-topic.

Sign in to view

@chorrell
Copy link
Contributor

chorrell commented May 18, 2018

Yep, merge away. Build is green and I'm happy with the shebang change.

@LaurentGoderre LaurentGoderre merged commit 9e2a121 into master May 18, 2018
@LaurentGoderre LaurentGoderre deleted the improve-coding-style branch May 18, 2018 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@LaurentGoderre LaurentGoderre LaurentGoderre approved these changes

@Starefossen Starefossen Awaiting requested review from Starefossen

@SimenB SimenB Awaiting requested review from SimenB

+1 more reviewer

@chorrell chorrell chorrell approved these changes

Reviewers whose approvals may not affect merge requirements

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

@PeterDaveHello @chorrell @LaurentGoderre