-
Notifications
You must be signed in to change notification settings - Fork 887
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
New tutorial #5871
New tutorial #5871
Conversation
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.
One small nit inline, but looks good otherwise!
Cheers, @s-makin
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.
Thanks for this PR @s-makin! I haven't reviewed the change yet, however it looks like updating the spelling work caused our CI spelling test to fail. You can run the test locally with this: tox -e doc-spelling -- doc/rtd doc/rtd_html
Currently this yields:
WARNING: doc/rtd/development/style_docs.rst:93: : Spell check: copybutton: Our documentation uses the Sphinx extension “sphinx-copybutton”, which creates.
WARNING: doc/rtd/howto/run_cloud_init_locally.rst:74: : Spell check: qemu: qemu-system-<arch>.
WARNING: ../../module-docs/cc_chef/data.yaml:1: : Spell check: validator: (string) The name of the chef-validator key that Chef Infra Client uses to access the Chef Infra Server during the initial Chef Infra Client run..
WARNING: ../../module-docs/cc_growpart/data.yaml:1: : Spell check: dev: (array of string) The devices to resize. Each entry can either be the path to the device’s mountpoint in the filesystem or a path to the block device in ‘/dev’. Default: .
WARNING: Found 4 misspelled words
Thanks, I didn't know about that - funnily enough, the CI test only showed up two of those four, while when I ran the test locally, I only got 3! Not sure what the cause of the inconsistency is, but I used your list to fix 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.
Thanks for this PR @s-makin! I left some comments inline.
|
||
Quick-start tutorial | ||
==================== | ||
* :ref:`Part 1: quick deployment <tutorial_lxd>` |
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.
A part 1 without a part 2 feels odd. Were you planning to add more here?
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 am yes, but it won't be part of this PR because it's still in the early draft stage.
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.
It's been agreed that Part 2 won't be done any time soon - would you prefer me to remove the reference, or should I leave it like this as a placeholder for where any future tutorials should go?
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.
Thanks @s-makin, this looks great!
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.
@s-makin I just noticed one minor issue - after that is fixed I'm happy to merge it!
Proposed Commit Message
Merge type