-
Notifications
You must be signed in to change notification settings - Fork 60
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
update documentation #57
Conversation
4aa6faf
to
c67637f
Compare
README.md
Outdated
To be able to successfully run all the tests in this repo, there are a couple of | ||
resources that need to be found in `/tmp`. To generate the resources at the | ||
expected locations, you can run the following command: | ||
`sudo ./../resources/make_test_resources.sh`. |
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.
You can also add that you need to cd resources
first and then run this command from there. I would also like to see how to run the tests (via cargo test
).
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.
Oops, I thought I'm in docs
, fixed! Also added more details about how to run the tests, thanks for the feedback!
3aa6976
to
ec3c982
Compare
pytest-3 <path_to_the_file> | ||
``` | ||
For example: | ||
```bash |
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.
nit: surround code snippets with blank lines (here and below)
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.
done, just curious, is this some kind of good practice? (I was actually trying to reduce the number of empty lines since it looks like they won't make any difference regarding the README file).
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.
Yep it's a not-exactly-official guideline from here, and also one of the things the mdl
markdown linter complains about
docs/DESIGN.md
Outdated
|
||
## Overview | ||
|
||
The reference VMM is composed of 2 major crates packaged in a binary | ||
application: the `vmm` and a oneshot CLI that invokes it. The `vmm` crate | ||
application: the `vmm` and an oneshot CLI that invokes it. The `vmm` crate |
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.
nit: a oneshot
is correct here, pronunciation-wise oneshot
sounds as if it started with the consonant w
(see a more compelling explanation)
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.
:O Just followed the IDE suggestion :(. I'll revert, thanks!
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.
done
resources/make_test_resources.sh
Outdated
TEST_RESOURCE_DIR="$(dirname "$SOURCE")" | ||
cd $TEST_RESOURCE_DIR | ||
|
||
./kernel/make_kernel_busybox_image.sh -f elf -k vmlinux-hello-busybox -w /tmp/vmlinux_busybox -j 2 |
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.
nit: you could set variables with the various locations in tmp
so that they're easier to modify; you could also check if the variable is already set and only modify it if it's unset (so that users who want different locations for the artifacts can do something like KERN_DIR=/home/user/kernel ./make_test_resources.sh
)
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.
done
The documentation was outdated. Added more details about the current state of vmm-reference. Also, added a script for running in one shot all the commands needed to get the test resources + some other small fixes. Signed-off-by: Laura Loghin <lauralg@amazon.com>
pytest-3 <path_to_the_file> | ||
``` | ||
For example: | ||
```bash |
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.
Yep it's a not-exactly-official guideline from here, and also one of the things the mdl
markdown linter complains about
No description provided.