Skip to content

Conversation

glennvl
Copy link
Contributor

@glennvl glennvl commented Oct 16, 2025

Add a github action to lint all the bash scripts.

I'm not sure if this is ok or if I'm out of line here since I had to make quite a bit of changes to the scripts to make the linter happy. The changes mostly have to do with the usage of double quotes and how conditions are written, there are no structural changes or changes to how everything works.

If we go through with this, which I personally think would be a good thing, I would like a thorough review of the changes first, preferably by multiple contributors. I also want to do further testing myself to make sure I did not unintentionally break anything before I consider this pull request ready to be merged.


Status

TODO list until no longer draft

  • Sanity check with podman
    • Build image
    • Launch zwift
  • Test build-image.sh script still works
  • Test entrypoint.sh script still works
    • Test with podman
    • Test with docker
  • Test install.sh script still works
  • Test run_zwift.sh script still works
  • Test update_zwift.sh script still works
  • Test zwift-auth.sh script still works
  • Test zwift.sh script still works
    • All parameters work with podman
    • All parameters work with docker

Copy link
Owner

@netbrain netbrain left a comment

Choose a reason for hiding this comment

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

I think this is a good idea, it will help us keep the scripts to a higher standard, and hopefully catch subtle bugs or syntax issues.

However the changes thus far might have som side effects. I've dotted down a few findings.

@glennvl glennvl marked this pull request as ready for review October 19, 2025 13:21
@glennvl
Copy link
Contributor Author

glennvl commented Oct 20, 2025

I tested what I can with podman. So if someone else can also test with docker that would be great.

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.

2 participants