-
Notifications
You must be signed in to change notification settings - Fork 52
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
DOCSITE: Reorganise tutorials and add inline solutions #231
base: master
Are you sure you want to change the base?
Conversation
In the preview you gave it looked great, good work!
Because this hasn't been done yet, I can't see what your changes to the tutorial files themselves are, making reviewing it difficult. I can do a manual diffs between _repos/sel4proj/sel4-tutorials/tutorials/* and Tutorials/*, but that is cumbersome. I used the following diff to build and preview the site locally: diff --git a/Gemfile b/Gemfile
index 9e06a50e54..363f7f04eb 100644
--- a/Gemfile
+++ b/Gemfile
@@ -27,6 +27,7 @@ group :jekyll_plugins do
gem 'jekyll-toc'
gem 'jekyll-redirect-from'
gem 'jekyll-remote-theme'
+ gem "jekyll-sass-converter", "~> 2.0" # TODO: Prevent 3.0 untill sass warnings are fixed
end
# Windows does not include zoneinfo files, so bundle the tzinfo-data gem
diff --git a/_config.yml b/_config.yml
index 65f4b8b9a2..b56c93e493 100644
--- a/_config.yml
+++ b/_config.yml
@@ -129,3 +129,4 @@ exclude:
- assets/css/highlighting/demos
- assets/css/highlighting/tools
- assets/css/highlighting/index.html
+ - Tutorials-old On the http://0.0.0.0:4000/Tutorials/ page, the two blocks on the right don't seem right and point to the wrong url? Maybe they shouldn't always be generated, but only on some pages where it makes sense. The "Tutorials about seL4" page has some broken urls (e.g. "About seL4"), mostly missing the new subdirectories you added for the tutorials, but other pages also have broken links. The "Quick solution" need a bit more margin below them and it would help if the mouse pointer would change when hovering above them, both can be fixed with a small css change. Also, when adding a new CSS rule, remove all existing styles from the HTML. It would be nice if, when clicking on a link in "How to: A quick solutions guide", the Quick solution would be expanded automatically. We can add this in the future, but it will require some Javascript (It needs to check a GET or POST variable and set All in all my first impression is that the new structure is a lot better and makes more sense compared to what we have now. I'll give more detailed feedback later, maybe when the changes are moved to the tutorials repo. |
Thanks!
I suggest waiting to review the tutorials until I've made the changes in the tutorials repo, then you'll be able to see line-by-line diffs.
I have added two new layouts to the Tutorial pages to remove the blocks that don't make sense.
The urls have been fixed.
TBD: Integrate CSS with markdown for quick solutions
TBD: Integrate js with markdown to expand solutions automatically
|
The tutorials have now been updated to fit into the new tutorial structure as per this PR (#231). The new tutorials: seL4/sel4-tutorials#106 A note on using the rewritten docsite (#231): Previous TODOs:
|
d06f44d
to
cd043b6
Compare
(rebased to more easily see the new commits) |
Ok, I've managed to get it working, and before I start reviewing all the small details, it's probably better to start high level:
|
Tutorials/index.md
Outdated
## How to use the tutorials | ||
|
||
Depending on your goals and what you want to do with seL4, we suggest | ||
different paths to follow through the tutorial material. Choose the | ||
most relevant category below and follow the tutorials in the suggested | ||
order. |
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 do think that section and set of paths was useful. We should not remove that completely, just recommend a default path (which the site navigation basically suggests now). There is a lot of useful content overview in here that we could use in the overview page.
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.
Much of this page has been put into the new Overview and Pathways pages, which describe the tutorials and alternate pathways respectively.
Tutorials/index.md
Outdated
<h2>Categories</h2> | ||
The tutorials are split into a number of broad categories: | ||
|
||
- [About seL4](GettingStarted/about-seL4) and [Getting started with the Microkit tutorial](GettingStarted/microkit) introduce seL4 concepts. | ||
- The [seL4 Kernel tutorials](seL4Kernel/overview.md) are a deep dive into seL4 concepts. | ||
- [MCS](MCS/mcs) introduces seL4 MCS extensions. | ||
- [Dynamic Libraries](DynamicLibraries/dynamic-1) covers the libraries that have been developed for rapidly prototyping systems on seL4. | ||
- [CAmkES](CAmkES/hello-camkes-0) is a platform for building componentised systems for embedded platforms. | ||
- [Microkit](https://trustworthy.systems/projects/microkit/tutorial/)is an operating system framework on top of seL4 provides a small set of simple abstractions that ease the design and implementation of statically structured systems on seL4. (Links to the same tutorial as in the [Getting Started](GettingStarted/microkit) section.) | ||
- [Rust](https://github.com/seL4/rust-sel4) provide crates for supporting the use of Rust in seL4 userspace. |
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 did not come across this content when I was navigating the new site -- which page does it get rendered to?
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.
This page was rendered when the user clicked on "Tutorials." I've now redirected "Tutorials" to overview, and used much of the above content in the overview page.
Tutorials/seL4Kernel/setting-up.md
Outdated
## Docker | ||
To compile and use seL4, it is recommended that you use Docker to isolate the dependencies from your machine. |
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.
If we are recommending docker, we don't really need the rest of the page apart from repo
(although I don't know if the virtual machine tutorials actually work under docker). We either should remove the docker section or add some text that makes clear that docker and the rest are two alternatives, and that you need only one of them.
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 actually removed everything but the repo and docker sections, because there is a whole other page on dependencies https://docs.sel4.systems/projects/buildsystem/host-dependencies.html, should someone need them.
I couldn't get of the virtual machine tutorials working on my silicon mac (but I don't think it's related to Docker at all).
Adding issues here for completeness:
"CAmkES VM: Adding a Linux Guest" uses KVM and I would have needed an alternative for mac.
"CAmkES VM: Cross VM Connectors" breaks using ninja
.
sh.ErrorReturnCode_1:
RAN: /usr/bin/cmake -G Ninja -DTUT_BOARD=pc -DTUT_ARCH=x86_64 -DFORCE_IOMMU=ON -DTUTORIAL_DIR=camkes-vm-crossvmke7jqji2 -C ../projec
STDOUT:
STDERR:
Should I add a note to these tutorials saying the instructions are for Linux only?
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.
Yes, that would be good. We've already had some questions from people who were trying to use other platforms.
Tutorials/seL4Kernel/setting-up.md
Outdated
## Getting a build environment | ||
To get a running build environment for seL4 and Camkes, run: | ||
|
||
```bash | ||
git clone https://github.com/seL4/seL4-CAmkES-L4v-dockerfiles.git | ||
cd seL4-CAmkES-L4v-dockerfiles | ||
make user |
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.
These are now docker instructions again. They should be in the docker section.
Signed-off-by: Birgit Brecknell <[email protected]>
Signed-off-by: Birgit Brecknell <[email protected]>
Signed-off-by: Birgit Brecknell <[email protected]>
Signed-off-by: Birgit Brecknell <[email protected]>
Signed-off-by: Birgit Brecknell <[email protected]>
Signed-off-by: Birgit Brecknell <[email protected]>
Co-authored-by: Gerwin Klein <[email protected]> Signed-off-by: bbrcknl <[email protected]>
Co-authored-by: Gerwin Klein <[email protected]> Signed-off-by: bbrcknl <[email protected]>
Co-authored-by: Gerwin Klein <[email protected]> Signed-off-by: bbrcknl <[email protected]>
Signed-off-by: Birgit Brecknell <[email protected]>
Signed-off-by: Birgit Brecknell <[email protected]>
Signed-off-by: Birgit Brecknell <[email protected]>
Signed-off-by: Birgit Brecknell <[email protected]>
Signed-off-by: Birgit Brecknell <[email protected]>
Signed-off-by: Birgit Brecknell <[email protected]>
Signed-off-by: Birgit Brecknell <[email protected]>
Signed-off-by: Birgit Brecknell <[email protected]>
Signed-off-by: Birgit Brecknell <[email protected]>
Signed-off-by: Birgit Brecknell <[email protected]>
Signed-off-by: Birgit Brecknell <[email protected]>
Signed-off-by: Birgit Brecknell <[email protected]>
Signed-off-by: Birgit Brecknell <[email protected]>
Signed-off-by: Birgit Brecknell <[email protected]>
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.
Other than some nitpicks, all looks good.
assets/js/toggle-markdown.js
Outdated
document.body.querySelectorAll('details') | ||
.forEach((e) => {(e.hasAttribute('open')) ? | ||
e.removeAttribute('open') : e.setAttribute('open',true); | ||
console.log(e.hasAttribute('open')) |
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.
This one is still unresolved.
Signed-off-by: Birgit Brecknell <[email protected]>
Signed-off-by: Birgit Brecknell <[email protected]>
Signed-off-by: Birgit Brecknell <[email protected]>
Signed-off-by: Birgit Brecknell <[email protected]>
Great, thanks for your review. I believe that all the outstanding issues have now been resolved. |
Signed-off-by: Indan Zupancic <[email protected]>
Signed-off-by: Birgit Brecknell <[email protected]>
fbae7e8
to
5fc2d96
Compare
Signed-off-by: Birgit Brecknell <[email protected]>
Aim: Arrange the tutorial material such that it is clear and easy to follow
Solution: Inspired by the Rust book https://doc.rust-lang.org/book/, the user can see the chapters in the index on the left, and go straight to the sections that they need
Specifically, updated tutorials with:
To-dos: