-
-
Notifications
You must be signed in to change notification settings - Fork 561
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
docs: added docker instructions to the exercises #3523
docs: added docker instructions to the exercises #3523
Conversation
@mrinalwadhwa no rush on this PR. I went through all the exercises just to make sure I didn't miss anything, but I would appreciate a second glance 😄 Feel free to suggest changes or provide feedback. I have more ideas on how we can improve the learning, but I think just getting out of the debugging users' local environment will be a good win, to begin with. |
@karl-cardenas-coding thank you for spending time on this! |
A couple of quick things, if you have time to improve them.
Thank you |
Editors/IDEs usually have builtin commands that do this, more helpful information here https://editorconfig.org/ |
@mrinalwadhwa nice catch, I'll get that fixed tomorrow. |
@mrinalwadhwa linting errors addressed ✔️ |
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.
Hi @karl-cardenas-coding, there's been some recent PR's merged into develop which add some minor modifications to the README.md
file that conflicts with your changes. Could you please add a commit to resolve the conflicts?
Thanks again for your time, your changes look great!
README.md
Outdated
Upon completion, you will be placed inside the `/work` folder of the container. Next, add a text editior for editing files. | ||
|
||
``` | ||
apt update && apt install nano |
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.
I didn't write the original guide but I would say that using touch
to create the files and not specifying any tool to edit those files is intentional so the user can proceed with their preferred tool (everyone has their own editing preferences after all). Any thoughts on this @mrinalwadhwa ?
@adrianbenavides I went ahead and rebased the PR. |
@Mergifyio rebase |
❌ Base branch update has failedGit reported the following error:
err-code: 8DBA3 |
I'm closing this PR as we have included a big change related to how we install/use the ockam binary, which makes most of the changes introduced here outdated. Thanks @karl-cardenas-coding for your work on this PR 🙏 |
Current Behavior
This PR addresses the challenges users may encounter when conducting the learning exercises in a local development environment. An example of an issue is #3521 , more examples can be found in the help thread.
Proposed Changes
The PR adds instructions on using a Docker container for the exercises featured in the main
README.md
. The user may still proceed with a local install should they choose to. The benefit is a better learning experience for the user and less time spent by the Ockam team on answering questions about a user's local environment, which can be difficult and time-consuming to debug.There is a price to pay with this setup. The first compile session will take a few minutes due to updates to crates.io index.
In my opinion, the tradeoff of less time spent in helping users debug local environment issues makes this worth it for all parties.
Checks