-
Notifications
You must be signed in to change notification settings - Fork 47
DOCS-3962: Tweak local module instructions #4330
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
Conversation
✅ Deploy Preview for viam-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
``` | ||
|
||
Then restart it in your machine's **CONFIGURE** tab in the Viam app. | ||
Restart the module in your machine's **CONFIGURE** tab in the Viam app. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so for pyinstaller I need to run the CLI command but for venv I need to do it in the UI? What happens if I use the restart option in the UI for pyinstaller and the cli command for venv?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So they're not actually the same--reload rebuilds AND restarts, whereas the UI just restarts. So, reload is unnecessarily time consuming if using run.sh as the entrypoint, because nothing needs to be built.
If you use the restart button for pyinstaller, you won't have a newly-built executable--you'll just be restarting the same executable you used previously.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think I should make this more explicit? Didn't want to over-explain and make it too verbose, but rather just tell them what they need to do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I think it may just be a bit of a tripwire where people see viam module build local
on the first tab and expect to be able to do the same on venv. I think a sentence about "Because you're using pyinstaller it is enough to rebuild the module with: ..." and soemthing similar on venv would avoid any stumbling
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay added something to the venv tab but I think the first tab is okay as-is now that the venv tab addresses why it's different.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First comment is not a blocker for this PR but one I think it's probably worth getting fixed in the product. Second comment is confusing to me
🔎💬 Inkeep AI search and chat service is syncing content for source 'Viam Docs' |
Apply some fixes that arose from a recent convo w/Felix