Skip to content

Conversation

@danieldean
Copy link
Contributor

This is a contribution for #273. Let me know if there are any issues!

All tests passed apart from doc building. It works when running for me and cleans up as expected.

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.

Great stuff, we really need some housekeeping.

However need to robustify the script a bit, and i think there's some mix of indentation usage (spaces). See comments.

@netbrain
Copy link
Owner

netbrain commented Feb 6, 2026

Another thing, the dont_clean option should also be present in flake.nix

@netbrain netbrain self-requested a review February 6, 2026 09:52
@glennvl
Copy link
Contributor

glennvl commented Feb 6, 2026

@danieldean The zwift.sh script was moved to a src directory in #272, so you need to rebase your branch before these changes can be merged (git rebase master).

@danieldean
Copy link
Contributor Author

@danieldean The zwift.sh script was moved to a src directory in #272, so you need to rebase your branch before these changes can be merged (git rebase master).

Thanks and noted. At the moment I think the array splitting is a blocker so will resolve that first if possible then rebase.

@danieldean danieldean requested a review from netbrain February 6, 2026 10:46
@danieldean
Copy link
Contributor Author

Ok... hopefully all issues resolved and all good now with the rebase. Bash is deceptively complicated!

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.

After this, i think it looks ok,

@danieldean
Copy link
Contributor Author

Ok, further suggested changes applied. Hope all is good now? I know some more Bash that I did to start with...

@netbrain
Copy link
Owner

netbrain commented Feb 9, 2026

Merging as is, it's fine.

@netbrain netbrain merged commit c9cbf67 into netbrain:master Feb 9, 2026
13 checks passed
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.

3 participants