Closed tomaarsen closed 3 years ago
finally got around to looking at this. it is BEAUTIFUL!!!!! Consider this an "Approval" since I don't seem to have the approve button available.
Here's what I especially like.
here are a few notes i have, but i'm a little rusty with web dev and happy to do it myself. i'd somewhat prefer merging as is considering this PR is already 170k lines and picking these sorts of things up as followups.
I'm very happy to see the positive response!
Regarding the possible changes:
Thanks for the detailed feedback! It's been very useful.
Great to see this feedback!
Unfortunately we don't have a favicon. Yes, we'll merge https://github.com/nltk/nltk/pull/2845 then rebuild the website from there.
Merged via 35f03a19c34d6c562f93f51a541c6ad82278d3c6. Glad to see the website up!
Hello!
Pull request overview
contribute.rst
into a separate file calledteam.rst
.Have a look at this website right here: https://tomaarsen.github.io/nltk.github.com This is based on https://github.com/tomaarsen/nltk.github.com/tree/modernised, which is identical to the branch I'm trying to merge here. With other words, the website linked above is what you'll get upon merging.
See https://github.com/nltk/nltk/pull/2845 for the documentation of this PR.
Additional Info
The website here was built for version 3.6.3, and it links to the 3.6.3 tag of NLTK in the footer. I would also recommend quickly finishing up NLTK 3.6.5 (perhaps I can help), after which we can create a new documentation version in a smoother fashion. To generate this documentation for version 3.6.3, I had to checkout 3.6.3 and copy over the
web
folder from my PR in NLTK - definitely not ideal. That will improve drastically once https://github.com/nltk/nltk/pull/2845 is merged, and we start releasing new versions.