Skip to content

Revamp docs #1628

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

Merged
merged 27 commits into from
May 17, 2020
Merged

Revamp docs #1628

merged 27 commits into from
May 17, 2020

Conversation

nhooyr
Copy link
Contributor

@nhooyr nhooyr commented May 12, 2020

  • Revamp README.md
  • Finish ./doc/guide.md
  • Clarify npm install deps

@nhooyr nhooyr requested a review from code-asher as a code owner May 12, 2020 05:40
@nhooyr nhooyr force-pushed the docs branch 2 times, most recently from 03ad916 to f656ecd Compare May 12, 2020 05:56
@@ -1,47 +1,93 @@
# code-server
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@code-asher only review README.md for now pls

@nhooyr nhooyr force-pushed the docs branch 2 times, most recently from e53f1ce to 67de202 Compare May 12, 2020 06:01
@nhooyr
Copy link
Contributor Author

nhooyr commented May 12, 2020

should split test into fmt, lint and test.

Copy link
Member

@code-asher code-asher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

README.md is looking good. I think we might want to do something about the hardcoded version number so users aren't copy-pasting and installing old versions. Maybe we just update the readme as part of the release process?

```bash
npm install -g code-server
code-server
```
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To match the others should we put the Now visit line here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opted against so that user's would read the output but I'll add for visual consistency.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense.

@nhooyr nhooyr force-pushed the docs branch 4 times, most recently from e775804 to f26337e Compare May 13, 2020 00:00
nhooyr added 3 commits May 12, 2020 21:26
- Splits up test into fmt, lint and test
- Fixes bug in build-packages.sh
- Minor README.md fixes
@nhooyr
Copy link
Contributor Author

nhooyr commented May 13, 2020

So idk wtf is going on with both npm/yarn, but you can't publish any node_modules folders in a module. So now we rename them all to node_modules.bundled and then on post install rename them back.

Also have to unset some environment variables for npm rebuild to work under npm -g. Now all works well :)

@nhooyr
Copy link
Contributor Author

nhooyr commented May 13, 2020

Punting windows release for now, this stuff took a lot longer than expected. Going to just finish this guide and then we should release.

@nhooyr
Copy link
Contributor Author

nhooyr commented May 13, 2020

  • If npm rebuild fails, should print a link to our npm docs

@nhooyr
Copy link
Contributor Author

nhooyr commented May 13, 2020

- [ ] Discuss 8443 as default port or enabling TLS by default and using 443 potentially 🤔

We'll do this with the move to the Go CLI and all the changes to be made there @code-asher

@nhooyr
Copy link
Contributor Author

nhooyr commented May 13, 2020

  • Move old extension directory on macOS

No good reason to use .zip, was just confusion on my part.
@nhooyr nhooyr force-pushed the docs branch 8 times, most recently from fe47544 to 977c27c Compare May 16, 2020 19:20
@nhooyr nhooyr force-pushed the docs branch 4 times, most recently from 9d70316 to 2b3eabb Compare May 17, 2020 22:00
@nhooyr nhooyr merged commit e955da1 into master May 17, 2020
@nhooyr nhooyr deleted the docs branch May 17, 2020 23:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants