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

Some docs cleanup #194

Merged
merged 21 commits into from
Feb 22, 2020
Merged

Some docs cleanup #194

merged 21 commits into from
Feb 22, 2020

Conversation

bollwyvl
Copy link
Collaborator

@bollwyvl bollwyvl commented Feb 11, 2020

References

Code changes

  • travis
    • 3.8 (not -dev)
    • add 3.9-dev
      • still getting the odd ModuleNotFoundError: No module named 'pip'

User-facing changes

Some more progress towards getting the docs usable, fixing links, etc.

Backwards-incompatible changes

N/A

Chores

  • linted
  • tested
  • documented
  • changelog entry

@bollwyvl
Copy link
Collaborator Author

Actually got the link checker wired up and passing locally... once this works in CI, that's probably a good place to stop!

@bollwyvl bollwyvl changed the title [wip] Some docs cleanup Some docs cleanup Feb 11, 2020
@bollwyvl
Copy link
Collaborator Author

LGTM

@bollwyvl bollwyvl requested a review from krassowski February 12, 2020 01:31
@@ -13,7 +13,7 @@
Language Server Specs can be [configured](./LANGUAGESERVERS.md) by Jupyter users,
or distributed by third parties as python or JSON files. Since we'd like to see
as many Language Servers work out of the box as possible, consider
[contributing a spec](../CONTRIBUTING.md#specs), if it works well for you!
[contributing a spec](../CONTRIBUTING.ipynb#specs), if it works well for you!
Copy link
Member

Choose a reason for hiding this comment

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

Confused by this...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

@@ -42,12 +42,12 @@ tested to work with `jupyter-lsp`.
Don't see an implementation for the language server you need? You can
[bring your own language server](#adding-custom-language-servers).

> Please consider [contributing your language server spec](../../CONTRIBUTING.md#spec)
> Please consider [contributing your language server spec](../Contributing.md)
Copy link
Member

Choose a reason for hiding this comment

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

this

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed


`pip`-installable packages in the same environment as the Jupyter `notebook` server
can be automatically detected as providing a language server spec. These are a
little more involved: see [CONTRIBUTING](../../CONTRIBUTING.md).
little more involved: see [CONTRIBUTING](Contributing.md).
Copy link
Member

Choose a reason for hiding this comment

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

and this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

docs/conf.py Outdated

html_context = {
"display_github": True,
"display_github": False,
Copy link
Member

Choose a reason for hiding this comment

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

IMO worth turning on (not necessarily in this PR)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

turned on

@krassowski
Copy link
Member

Looks good in general, just want to get a confirmation about the paths to Contributing. By my intuition, at least one of them is must be wrong (but I can be wrong too!).

@bollwyvl
Copy link
Collaborator Author

All of the above are link-checker-driven-development artifacts, whoops:

  • i think i can fix all the internal links: (nb)sphinx is strangely passive aggressive about file extensions, in that sometimes it doesn't even care which you use, and sometimes gets very angry
    • might end up rewriting more markdown getting passed through to notebooks, or see if i can get rid of the wrappers altogether (preferable)
  • the github link creates links on generated pages that don't exist, like search and genindex
    • i'll look into not showing those on those specific pages

@bollwyvl
Copy link
Collaborator Author

Well, that ended up being a bit more work than i was hoping, but it's feeling better:

  • fixed the the search and genindex github links
    • ...rather brutally by forking the template (ignored by prettier)
  • moved all the docs content into notebooks
  • generate the language server tables directly from a hot manager instance 🎉
    • requires doing the full yarn and python install, but we'll KNOW the docs are right (if they build)

I guess at this point with as much content in there as there is, it may well be worth it to grab some of the stuff from prenotebook as part of lint.py:

  • black/isort the python code cells
  • prettier the markdown
  • strip the outputs/execution numbers

{% endblock %}
{% block breadcrumbs_aside %}
<li class="wy-breadcrumbs-aside">
{% if hasdoc(pagename) and pagename not in ["search", "genindex"] %}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

all for this line...

README.md Outdated
@@ -4,11 +4,11 @@

> _This project is still maturing, but you are welcome to check it out, leave feedback and/or a PR_

Quick Links: **[Installation](#installation) | [Language Servers](./docs/LANGUAGESERVERS.md) | [Updating](#updating) | [Changelog](./CHANGELOG.md) | [Roadmap](./docs/ROADMAP.md) | [Contributing](./CONTRIBUTING.md) | [Extending](./docs/EXTENDING.md)**
Quick Links: **[Installation](./Installation.ipynb) | [Configuring](./docs/Configuring.ipynb) | [Updating](#updating) | [Changelog](./CHANGELOG.md) | [Roadmap](./docs/Roadmap.ipynb) | [Contributing](./CONTRIBUTING.md) | [Extending](./docs/Extending.ipynb)**
Copy link
Member

Choose a reason for hiding this comment

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

./docs/Installation.ipynb ? Actually, I would probably prefer to have this go to the short version of the installation within README (we can shorten it) and then to have a link to the notebook from here.

In the quick links we maybe could have something like: Installation: [Basic](#installation) | [Full](notebook) (but I am not full convinced)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

agreed, README.md needs to pretty much be standalone for the 80% case. Hopefully fixed now.

@bollwyvl
Copy link
Collaborator Author

Thanks for the review, looking into it now...

@bollwyvl
Copy link
Collaborator Author

Probably best if this lands prior to #199 so that i can fix up changelog and extending while it's still fresh in my mind... so many moving pieces.

@bollwyvl bollwyvl closed this Feb 18, 2020
@bollwyvl bollwyvl reopened this Feb 18, 2020
@bollwyvl bollwyvl closed this Feb 19, 2020
@bollwyvl bollwyvl reopened this Feb 19, 2020
@krassowski krassowski closed this Feb 22, 2020
@krassowski krassowski reopened this Feb 22, 2020
@krassowski
Copy link
Member

We will need to do something about these Linux/Docker failures...

@krassowski krassowski merged commit 685504d into jupyter-lsp:master Feb 22, 2020
```bash
conda install -c conda-forge nodejs
# or one of the following, as an administrator
choco install nodejs # Windows with Chocolatey
Copy link
Member

Choose a reason for hiding this comment

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

if it is all in one block, I am 100% sure some users will just copy-paste it all. I would move these examples to the docs.

```bash
conda install -c conda-forge python=3
# or
conda install -c conda-forge jupyterlab=1.2
Copy link
Member

Choose a reason for hiding this comment

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

?

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