E0d/update first pr #858
E0d/update first pr #858
Conversation
| python3.8 -m venv tutor-venv | ||
| . tutor-venv/bin/activate | ||
| .. admonition:: The Tutor documents will recommend this as well, but | ||
| please do yourself the favor or using python virtual |
There was a problem hiding this comment.
Grammatical mistake - "of" rather than "or".... "please do yourself the favor of using python virtual..."
| Depending on your system and your Internet connection speed, | ||
| this could take anywhere from five minutes to over an hour, | ||
| so go get a coffee and come back for the next part. | ||
| `Official Tutor tutorial <https://docs.tutor.edly.io/tutorials/main.html#running-open-edx-on-the-master-branch-tutor-main>`_ |
There was a problem hiding this comment.
I'm still wrapping my head around the different types of Tutor - Main, Full, Nightly(?). I did find that this link was the most straight forward as far as just getting it installed as a python package (although it does mention other ways): https://docs.tutor.edly.io/install.html#install
| checkout: | ||
| To set up your local enviroment to update edx-platform, follow the | ||
| `official instructions | ||
| <https://docs.tutor.edly.io/tutorials/edx-platform.html>`_ |
There was a problem hiding this comment.
I know this was referenced in the forums discussion, but after going through it, I found it not as helpful for the following reasons:
- it has some grammatical errors "edx-plaform" instead of "edx-platform" on the workspace path.
- the confusion of using the named release versus master depending on which Tutor you're using
- "This command does a few "magical" things behind the scenes" - I believe it's missing the actual command which may be
tutor dev launchbecause I'm confidenttutor mounts adddoes not do what it's explaining. - it tells you to
tutor images build openedxinstead of building openedx-dev. I may be learning still, but I was never able to see my changes reflected on the screen when I built the non "-dev" image.
I would suggest that consolidating the link you mentioned and this one: https://docs.tutor.edly.io/dev.html#first-time-setup so it's more clear on what commands are to be run in order to get your instance running, and get your changes on the legacy platform reflected locally.
There was a problem hiding this comment.
When you say tutor mounts add does not do what it's explaining, are you referring to this section?
Similarly, when developing a custom XBlock, we would like to hot-reload any change we make to the XBlock source code within the containers.
Tutor provides a simple solution to these questions. In both cases, the solution takes the form of a tutor mounts add ... command.
There was a problem hiding this comment.
No, further down in the docs:
Although re-reading it I have misunderstood that this command is meant to be used in combination with the others since it bind mounts the edx-platform image for build-time or at run-time.
There was a problem hiding this comment.
Submitted a PR to fix the "edx-plaform" typo.
@tonybusa, if you still have specific suggestions on how to improve those docs, contributions would be very welcome! :)
|
|
||
| python3.8 -m venv tutor-venv | ||
| . tutor-venv/bin/activate | ||
| .. admonition:: The Tutor documents will recommend this as well, but |
There was a problem hiding this comment.
I don't know why this isn't working but I don't use this directive, I use .. note:: or .. warning::
But, maybe the following text is indented wrong? You could just soft-wrap the line.
There was a problem hiding this comment.
@sarina sorted it out. A combination of how the admonition title works and how the details are formatted.
There was a problem hiding this comment.
Actually, noticing that admonition provides 0 styling, which isn't super useful. Does warning or another option provide styling?
|
@brian-smith-tcril or @arbrandes could you suggest a simple, but useful change to the MFE that would demonstrate the integration? A link out would also be great. Also please check my instructions for correctness. |
There was a problem hiding this comment.
Looks great! Made a few suggestions.
As for suggesting simple changes in the docs, wouldn't it be better to document (or just link to) how to develop a useful plugin using the dev env instead? Such as the plugin docs in the tutor-mfe README, but geared more for the dev environment. I could write one up as a subsequent PR.
| checkout: | ||
| To set up your local enviroment to update edx-platform, follow the | ||
| `official instructions | ||
| <https://docs.tutor.edly.io/tutorials/edx-platform.html>`_ |
There was a problem hiding this comment.
Submitted a PR to fix the "edx-plaform" typo.
@tonybusa, if you still have specific suggestions on how to improve those docs, contributions would be very welcome! :)
| Not every MFE currently has a dev profile that will | ||
| work with Tutor, though it is possible to create one if that is the | ||
| case for the MFE you are developing. |
There was a problem hiding this comment.
| Not every MFE currently has a dev profile that will | |
| work with Tutor, though it is possible to create one if that is the | |
| case for the MFE you are developing. | |
| Not every MFE currently has a dev profile that will | |
| work with Tutor, though it is possible to create one if that is the | |
| case for the MFE you are developing, using `an existing one <https://github.com/openedx/frontend-app-learner-dashboard/pull/530/files>`_ as a template. |
Co-authored-by: Adolfo R. Brandes <arbrandes@arbrand.es>
Co-authored-by: Adolfo R. Brandes <arbrandes@arbrand.es>
Co-authored-by: Adolfo R. Brandes <arbrandes@arbrand.es>
Co-authored-by: Adolfo R. Brandes <arbrandes@arbrand.es>
Co-authored-by: Adolfo R. Brandes <arbrandes@arbrand.es>
| Working with the Learner Dashboard MFE | ||
| ====================================== | ||
|
|
||
|
|
There was a problem hiding this comment.
@arbrandes thoughts about a change that makes sense to kick the tires? I updated a string in messages.json which I didn't think we great, it would be better to change a little react code I think.
|
Linked some issues I know this will resolve; it might also resolve #103 |
WIP PR to update the first PR docs to be: