Readme cleanup, docs URL and CONTRIBUTING.md #1010
Conversation
There was a problem hiding this comment.
Thank you for moving the readme and adding the figures @romilbhardwaj ! It looks good to me.
| * Examples | ||
| * Documentation | ||
| * Tutorials, blog posts and talks on SkyPilot | ||
|
|
There was a problem hiding this comment.
Add: Please read through this document before contributing so that there is all the necessary information needed for us to effectively respond to your contribution.
There was a problem hiding this comment.
Good point - makes the flow better. Thanks!
There was a problem hiding this comment.
This sentence unfortunately reads a bit too direct to me. I'd love to not include it. It is implied anyway.
| * Documentation | ||
| * Tutorials, blog posts and talks on SkyPilot | ||
|
|
||
| ## Contributing Code |
There was a problem hiding this comment.
Bit of confusion between the bullet points mentioned in Contributing to SkyPilot and the bullet points below. I thought the doc would be structured as:
Contributing Bug ReportsContributing Pull RequestsContributing Test cases- etc
There was a problem hiding this comment.
I thinking maybe change the intro section to match the logical structure below.
| - `export SKYPILOT_MINIMIZE_LOGGING=1` to minimize the logging for demo purpose. | ||
|
|
||
| ### Testing | ||
| To run smoke tests: |
There was a problem hiding this comment.
Also, there is extra work needed to install Testing dependencies. Good to add that too.
There was a problem hiding this comment.
Hmm, those should be covered in pip install -r requirements-dev.txt above.. are there any other dependencies?
| bash tests/run_smoke_tests.sh test_minimal | ||
| ``` | ||
|
|
||
| For profiling code, use: |
There was a problem hiding this comment.
Maybe delete. First time I have seen this profiling text on a Contribution md file.
There was a problem hiding this comment.
I think it's generally useful to have these snippets here and the contributing guide can be verbose in the interest of completeness. Also okay to remove if you feel strongly otherwise
…iments into readme_oss � Conflicts: � README.md
romilbhardwaj
changed the title
There was a problem hiding this comment.
Thanks for this @romilbhardwaj! Minor comments; otherwise mostly LGTM.
| * Examples | ||
| * Documentation | ||
| * Tutorials, blog posts and talks on SkyPilot | ||
|
|
There was a problem hiding this comment.
This sentence unfortunately reads a bit too direct to me. I'd love to not include it. It is implied anyway.
CONTRIBUTING.md
Outdated
| ``` | ||
| IMPORTANT: Please `export SKYPILOT_DEV=1` before running the CLI commands in the terminal, so that developers' usage logs do not pollute the actual user logs. | ||
|
|
||
| ### Environment variables for developers |
There was a problem hiding this comment.
Proposal: how about we move this to the end of this doc? It's more important to read about things like Testing, style guide, etc.
Section order that feels more natural to me (from here on):
- Submitting pull requests
- Testing
- Some general engineering practice suggestions
- Environment variables for developers
There was a problem hiding this comment.
Sounds good! I do want to keep testing before submitting pull requests, so the order now is:
- Installing
- Testing
- Submitting pull requests
- Some general engineering practice suggestions
- Environment variables for developers
README.md
Outdated
| @@ -1,117 +1,69 @@ | |||
| # SkyPilot | |||
|
|
|||
|  | |||
There was a problem hiding this comment.
We can use some markdown magic to limit the width? Currently it feels too wide compared to size of the text. E.g., https://github.com/ray-project/ray/
There was a problem hiding this comment.
Done! Set to 80% width
README.md
Outdated
|
|
||
|  | ||
| [](https://skypilot.readthedocs.io/en/latest/?badge=latest) | ||
|
|
||
| SkyPilot is a framework to run any workload seamlessly across different cloud providers through a unified interface. No knowledge of cloud offerings is required or expected – you simply define the workload and its resource requirements, and SkyPilot will automatically execute it on AWS, Google Cloud Platform or Microsoft Azure. | ||
| SkyPilot is a framework to run machine learning[^1] workloads seamlessly across different cloud providers through a unified interface. No knowledge of cloud offerings is required or expected – you simply define the workload and its resource requirements, and SkyPilot will automatically execute it on AWS, Google Cloud Platform or Microsoft Azure. |
There was a problem hiding this comment.
Can we change this first line to match our docs
SkyPilot is a framework for easily running machine learning projects on any cloud through a unified interface.
(with the footnote added)? A few words shorter. "workloads" vs "projects": either sounds good to me.
Cleaning up
README.mdfor open source release and added andCONTRIBUTING.md.TODO:
Fix the huge SVG animation to be a gif insteadDo in next PRAdd SVG/asciinema cast file to repo?Do in next PR