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

Modify install script to fail if the read-only filesystem is enabled #1250

Conversation

cghague
Copy link
Contributor

@cghague cghague commented Jan 4, 2023

Resolves #1243 by modifying the install script to fail if the read-only filesystem is enabled.
Review on CodeApprove

Copy link
Contributor

@mtlynch mtlynch left a comment

Choose a reason for hiding this comment

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

Automated comment from CodeApprove ➜

⏳ Approval Pending (1 unresolved comments)
Approval will be granted automatically when all comments are resolved

LGTM, thanks!


In: bundler/bundle/install:

> Line 54
  echo 'See https://tinypilotkvm.com/faq/read-only-filesystem for details.' >&2

Can we link to the exact heading?

https://tinypilotkvm.com/faq/read-only-filesystem#disabling-the-read-only-filesystem


👀 @cghague it's your turn please take a look

Copy link
Contributor Author

@cghague cghague left a comment

Choose a reason for hiding this comment

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

Automated comment from CodeApprove ➜

In: bundler/bundle/install:

> Line 54
  echo 'See https://tinypilotkvm.com/faq/read-only-filesystem for details.' >&2

I had that at first but the extra text takes the link to 84 characters which is slightly over the typical terminal width. The resulting line break doesn't look great and makes copying it from a terminal window awkward. Would you still like me to make this change?


👀 @mtlynch it's your turn please take a look

Copy link
Contributor

@mtlynch mtlynch left a comment

Choose a reason for hiding this comment

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

Automated comment from CodeApprove ➜

Approved on CodeApprove
✔️ Approved

LGTM, thanks!


In: bundler/bundle/install:

> Line 54
  echo 'See https://tinypilotkvm.com/faq/read-only-filesystem for details.' >&2

Oh good point. Leave as-is then.


👀 @cghague it's your turn please take a look

@cghague cghague merged commit 289b85a into master Jan 5, 2023
@cghague cghague deleted the 1243-fail-install-script-when-read-only-filesystem-is-enabled branch January 5, 2023 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fail install script when read-only filesystem is enabled
2 participants