Skip to content
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

[WIP] New docs #84

Closed
wants to merge 4 commits into from
Closed

[WIP] New docs #84

wants to merge 4 commits into from

Conversation

astorije
Copy link
Member

@astorije astorije commented Jun 12, 2018

This is still a work in progress, but keeping it as an open PR as we had people wonder why the docs don't reflect the current state of the project. This will also generate a preview for whoever wants to access them (at your own risk :D).

Definitely not ready for review, so don't waste your time on it yet :)

Closes #42. Extracted to #42
Closes #56. Extracted to #134
Closes #57. Nope
Closes #3. Extracted to #91
Closes #24. Extracted to #119
Closes #71. Extracted to #116.
Closes #83. Extracted to #104

TODO

Won't fix

  • Color contrast on links are not AA compliant: Will do on re-style post-docs

@AlMcKinlay
Copy link
Member

An additional thing we need to add to the todo list: theme/package instructions. You've got the API but not instructions on installing/using.

@astorije
Copy link
Member Author

astorije commented Sep 28, 2018

@McInkay, thanks for checking out! :)
I did add something actually, in the Usage section: https://deploy-preview-84--thelounge-chat.netlify.com/docs/usage#installing-additional-themes

screen shot 2018-09-27 at 23 48 08

At the moment, it's only about themes because there are no packages out there at all, but as soon as https://www.npmjs.com/search?q=keywords%3Athelounge-package has a single package, I'll update the section to be about both themes and packages :)
Chicken and egg situation, because it could also be confusing to tell users how to install packages when there is not a single one.
I am confident you will be publishing the very first package, so I'm happy to work on the doc if you do so!

@AlMcKinlay
Copy link
Member

I have a number of packages ready to be published once my PRs are merged :-P so yeah, that's fine leaving it as just themes just now, I get that.

However, I'm wondering whether that's obvious enough or not. Maybe it is.

@xPaw
Copy link
Member

xPaw commented Sep 28, 2018

It should be noted in install docs that since Yarn installs into user folder by default, the binary isn't going to be available in $PATH.

On linux the fix is to add Yarn global path to your $PATH like export PATH="$PATH:$(yarn global bin)"

EDIT: Actually it is mentioned, but that one liner is probably preferable to be included as well.

@astorije
Copy link
Member Author

astorije commented Oct 1, 2018

@xPaw, I'd rather not do this because it's very platform-dependent. For example, on macOS (though I know server install is rarely on macOS, but still, we do advertise it as cross-OS :D), it's available by default if you install with Homebrew.

You probably remember, but long time ago (ahem) we were documenting how to install Node on Ubuntu, Mac, and Windows, then we had to add CentOS/Fedora/RHEL, at which point we decided to redirect to Node's comprehensive package manager install document (as it is on the current doc). The Yarn install page is actually really good, lets you pick your OS and gives instructions accordingly, so I think I'd rather send reader to this page early in the install section, similarly to what we do with Node. You'd be okay with that?

@xPaw
Copy link
Member

xPaw commented Oct 1, 2018

You're right their install is pretty good. I just think we can make the npm install section a little easier to follow. For example we don't link to that page on how to install Yarn.

@astorije
Copy link
Member Author

astorije commented Oct 1, 2018

Yep, that's what I meant, adding a link to this page and making it all a bit better. Will do!

_config.yml Outdated
@@ -6,14 +6,17 @@ include:
- _redirects
kramdown:
toc_levels: "2"
include:
- _redirects
Copy link
Member

Choose a reason for hiding this comment

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

dupe

@astorije
Copy link
Member Author

astorije commented Nov 4, 2018

Closing this PR now. Everything has been moved to other PRs/issues and in most cases merged. Only leftovers of this PR are things that do not need to be merged.

@astorije astorije closed this Nov 4, 2018
@astorije astorije deleted the astorije/better-docs branch November 4, 2018 22:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment