-
Notifications
You must be signed in to change notification settings - Fork 5
feat: New Landing Page and Sidebars, Documents Updates #139
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
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.
There are a couple of changes that you need to make. About half of them are either me nitpicking or formatting errors.
One consistent observation I made was that you relied on the previous content too much and did not verify the commands. Half of the commands are no longer working. By testing the commands yourself, you could have verified the authenticity of the commands and known that the documentation actually works and new people can understand what they need to do with it.
If you are having issues understanding it yourself, then it would be difficult for others to understand it as well.
#### Unix-like Systems Example | ||
|
||
```bash | ||
polykey secrets env --env-format unix --env Weather-Ops:API_KEY | ||
``` | ||
|
||
#### JSON Format Example | ||
|
||
```bash | ||
polykey secrets env --env-format json --env Weather-Ops:API_KEY | ||
``` |
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.
Give output example here instead of examples of writing a command. The secrets don't have to actually store important data, they can just store some gibberish text.
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 think i've fixed this now.
```bash | ||
polykey secrets env --env AWS-Creds:AWS_ACCESS_KEY_ID,AWS-Creds:AWS_SECRET_ACCESS_KEY -- aws s3 ls | ||
``` |
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'm not sure if separation works by using a comma. Instead, specify multiple env paths before the command.
$ polykey secrets env aws-creds:AWS_ACCESS_KEY_ID aws-creds:AWS_SECRET_ACCESS_KEY -- aws s3 ls
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.
ok done
The Polykey core contains the service, domain business logic and persistence | ||
layers of Polykey. | ||
|
||
It is written in TypeScript and C++. |
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 is also no longer true. Polykey is basically completely written in typescript. Only supporting libraries have platform-native C++/Rust code.
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.
ok fixed
Install it with: | ||
|
||
```shell | ||
npm install --location=global polykey |
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.
Polykey is just library code and will not have any executable code. Instead, they need to install polykey-cli
, but that is already covered in previous sections. We might need to have a quick chat to discuss this page a bit more, as most of this page is severly out of date.
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.
ok i've fixed it but lets talk and see if I'm correct in my fix
I see alot of review comments identifying things that you should have been able to identify yourself just by testing the commands on your own computer @xrissoula. We've had a meeting to discuss this along with filling in some computer architecture context, so in the future things like |
Are you going to change the branch name of this PR feat-docs-landing-page? |
Closed in favour of #140. |
Description
This PR revamps our docs landing page and updates the Getting Started section, addressing outdated content from previous PRs and issues. It reorganizes the documentation according to the Divio principles and merges or removes obsolete tutorial information. Key highlights include:
These changes incorporate feedback from the old PR and aim to provide a more streamlined, user-friendly introduction to Polykey.
Tasks
Final checklist