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

Reorganise docs #279

Merged
merged 4 commits into from
Mar 13, 2017
Merged

Reorganise docs #279

merged 4 commits into from
Mar 13, 2017

Conversation

seanh
Copy link
Contributor

@seanh seanh commented Mar 10, 2017

The docs are built here: http://client.readthedocs.io/en/reorganise-docs/

You can also build them locally: http://client.readthedocs.io/en/reorganise-docs/documenters/buildthedocs/

This PR moves the docs directory to Sphinx.

Docs are also reorganized by audience.

Some docs (for example embedding.rst and authorization-grant-tokens.rst) are copied over from the h repo's docs. These will be removed from the h repo after this commit is merged.

Keep all the docs for publishers (i.e. people who run websites that
publish content, and want to embed the h client into their site)
together in one docs/publishers folder.
These are documentation files intended for publishers who're embedding h
in their sites, they're being removed from the h repo into this client
repo.

There may be some reStructuredText syntax in these files that doesn't
work on GitHub (only in Sphinx or only on readthedocs) but these client
docs are about to be moved over to Sphinx/readthedocs anyway.
@seanh seanh added the WIP label Mar 10, 2017
@seanh seanh force-pushed the reorganise-docs branch 2 times, most recently from 91230f4 to d5c7f0a Compare March 10, 2017 15:28
@seanh seanh added Needs Review and removed WIP labels Mar 10, 2017

# General information about the project.
project = u'Hypothesis Client'
copyright = u'2017, Hypothesis Project Contributors'
Copy link
Member

Choose a reason for hiding this comment

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

Do we actually need the year or could we leave it out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dunno, this is autogenerated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ugh, there is disagreement over whether a copyright notice on a website should include the year or not, and whether the year should be updated over time. I'm inclined to leave this as it was generated.

docs/conf.py Outdated
# The short X.Y version.
version = u'1.2.0'
# The full version, including alpha/beta/rc tags.
release = u'1.2.0'
Copy link
Member

Choose a reason for hiding this comment

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

Could this be read from the package version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The autogeneration tool requires a version num to be given (not sure if it's actually needed, maybe not, maybe it is) but it's not actually used anywhere (as with h, on rtd it'll show as just one version called latest). It may be possible to pull it from somewhere - this is a Python file that can contain arbitrary code - we'd have to make sure that ran on readthedocs too, but it should be doable. (I'd say don't bother though - we aren't using this number.)

Copy link
Member

Choose a reason for hiding this comment

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

OK. If it isn't used as far as you know, can you just set it to "1.0.0" to make it obvious that it isn't meant to be the same as the package version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed it to 1.0.0

htmlhelp_basename = 'HypothesisClientdoc'


# -- Options for LaTeX output ---------------------------------------------
Copy link
Member

Choose a reason for hiding this comment

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

I think I'd prefer to remove all the non-HTML stuff until we're sure someone wants it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think RTD may use this to generate the PDF copy of the docs (see for example the h docs in PDF), but not sure. It's just an autogenerated file, generated using the commands that RTD's docs give, I made only the minimal modifications necessary to it. We can try to cut it down to the minimum if you want but I don't think it's worth the effort.

Copy link
Member

Choose a reason for hiding this comment

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

OK. I'll leave it up to you.

Environment Variables
=====================

This section documents all the environment variables supported by the client.
Copy link
Member

Choose a reason for hiding this comment

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

The client itself doesn't see or do anything with env vars. What you mean is that these env vars affect the behaviour of the client's build tasks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed this

@@ -0,0 +1,146 @@
Generating Authorization Grant Tokens
Copy link
Member

Choose a reason for hiding this comment

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

The auth grant tokens are a feature of the service rather than the client. I would be inclined to leave the documentation in the service and include just a suitable link from the client docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I've replaced this with an intersphinx link to the section in the h docs, but that link will be broken until hypothesis/h#4423 is merged

This is one we're gonna have to watch out for - the fact that our code is split into separare repos for web service, client, browser extension etc is an implementation detail that the documentation's audience might not want to be bothered with. Or is it? Anyway, we probably want to avoid distributing docs for one audience across multiple sites too much. But when we do have to, intersphinx should be helpful.

import os
# import sys

from recommonmark.parser import CommonMarkParser
Copy link
Member

Choose a reason for hiding this comment

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

It looks like you've translated all the docs from Markdown to rST. In that case we could leave out the markdown support for the moment.

Copy link
Contributor Author

@seanh seanh Mar 10, 2017

Choose a reason for hiding this comment

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

Shall we just leave it in? Then people can go ahead and write one file in markdown, or directly import an existing markdown file, if they don't have time right now to figure out rst or to translate markdown to rst (and as long as they don't need to use any rtd or sphinx features that markdown doesn't have). I think it's potentially handy for quick things although we may end up not using it.

@@ -0,0 +1,20 @@
Building the Docs Locally
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure that I would consider "documenters" to be a separate role from developers at the moment. In future I can imagine that we will have Translators as a distinct role.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it makes the structure and navigation clearer to have the docs to do with coding and the docs to do with documenting separated. Otherwise the docs about docs are buried at the bottom of the chapter about code, where nobody would expect to look for them. There'll be a couple more files to add to this directory later.

Change the docs directory (which was just a directory of markdown files)
to a reStructuredText / Sphinx project.

Docs are reorganized by audience.

Some docs (for example embedding.rst) are copied over from the h repo's
docs. These will be removed from the h repo after this commit is merged.
@seanh
Copy link
Contributor Author

seanh commented Mar 13, 2017

The docs are back up, after the URL change, here: http://h-client.readthedocs.io/en/reorganise-docs/

Copy link
Member

@robertknight robertknight left a comment

Choose a reason for hiding this comment

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

The links to the development guide in README.md need to be updated to point to the URL where this content will appear. Otherwise I think this is pretty much good to merge.

@robertknight
Copy link
Member

We agreed on Slack that the README change would happen in a follow-up PR.

@robertknight robertknight merged commit b017a5b into master Mar 13, 2017
@robertknight robertknight deleted the reorganise-docs branch March 13, 2017 11:06
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.

None yet

2 participants