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

Update docs #3498

Merged
merged 19 commits into from Feb 18, 2018

Conversation

Projects
None yet
4 participants
@stsewd
Member

stsewd commented Jan 9, 2018

Probably closes:

@stsewd

This comment has been minimized.

Member

stsewd commented Jan 9, 2018

The most relevant commits are:

Note: the whole docs needs to be re-wrapped to 80 chars, for now I only did this for the files I touched.
Note2: This should be updated too (probably another PR) https://github.com/rtfd/readthedocs.org/blob/master/readthedocs/doc_builder/base.py#L89 (the text of the autogenerated index file and the logic when to generate this file)

@stsewd stsewd referenced this pull request Jan 9, 2018

Open

README.md not found #1615

@ericholscher

This comment has been minimized.

Member

ericholscher commented Jan 10, 2018

I'm generally -1 on breaking docs at 80 characters. I'd much prefer to have them broken on semantic meaning (eg. periods or commas). This has a bit more background here: http://rhodesmill.org/brandon/2012/one-sentence-per-line/, but I feel pretty strongly against just doing a straight "80 char" break.

@ericholscher

This comment has been minimized.

Member

ericholscher commented Jan 10, 2018

I'd also ask that we not refactor content when we also change it in the PR. It makes it almost impossible to see what actually changed, so almost impossible to review.

@ericholscher

This comment has been minimized.

Member

ericholscher commented Jan 10, 2018

In general I love docs changes though! Thanks for doing these, and I'll try and figure out what exactly is going on, so I can review it.

@ericholscher

This comment has been minimized.

Member

ericholscher commented on docs/builds.rst in d907524 Jan 10, 2018

+1

@ericholscher

This comment has been minimized.

Member

ericholscher commented Jan 10, 2018

Also, we should definitely have a doc style guide, to answer questions like these :)

@RichardLitt

This comment has been minimized.

Member

RichardLitt commented Jan 11, 2018

@stsewd Can you add the doc styleguide in this PR? It can probably go in the CONTRIBUTE doc on the website if it is small, at this point.

@ericholscher

This comment has been minimized.

Member

ericholscher commented Jan 11, 2018

We can probably steal a decent bit from here: https://github.com/writethedocs/www/blob/master/docs/style-guide.rst

@stsewd

This comment has been minimized.

Member

stsewd commented Jan 12, 2018

@RichardLitt I added some guidelines as suggested :). I feel like we need to improve a little more the section for contributing to the docs (and putted it a little closer to the Contributing to Read the Docs section), but probably another PR.

@ericholscher you are right, breaking the lines at semantic meaning rather that 80 chars is a good idea, the explanation from the post is very clear. Should I revert the other commits then?

stsewd added some commits Jan 23, 2018

@@ -37,8 +37,6 @@ We will look inside a ``doc`` or ``docs`` directory first,
and then look within your entire project.
Then Sphinx will build any files with an ``.rst`` extension.
If you have a ``README.rst``,
it will be transformed into an ``index.rst`` automatically.
Mkdocs

This comment has been minimized.

@stsewd

stsewd Jan 23, 2018

Member

Should be MkDocs? (uppercase D)

This comment has been minimized.

@humitos

humitos Feb 15, 2018

Member

Yeah, that's the correct way of writting it: http://www.mkdocs.org/

@stsewd

This comment has been minimized.

Member

stsewd commented Jan 24, 2018

I reverted the noisy commits and applied the style guide as mentioned, this is ready for another review.

@humitos

Nice! Thanks @stsewd. I left a couple of comments for you to take a look :)

@@ -10,4 +10,29 @@ You can build the docs by installing ``Sphinx`` and running::
# in the docs directory
make html
Let us know if you have any questions or want to contribute to the documentation.
Please follow these guidelines when updating our docs.
Let us know if you have any questions or if something isn't clear.

This comment has been minimized.

@humitos

humitos Feb 15, 2018

Member

Remove the if from "if something"

-------
* Do not break the content across multiple lines at 80 characters,
but rather broken them on semantic meaning (e.g. periods or commas).

This comment has been minimized.

@humitos

humitos Feb 15, 2018

Member

break them

This comment has been minimized.

@humitos

humitos Feb 15, 2018

Member

Also, I think it would be awesome to link the post that Eric shared with you. You said that was useful to understand this idea, so may be good to link it here.

@@ -57,7 +57,7 @@ environment, and will be set to ``True`` when building on RTD::
{% endif %}
I get import errors on libraries that depend on C modules
----------------------------------------------------------
---------------------------------------------------------

This comment has been minimized.

@humitos

humitos Feb 15, 2018

Member

Did you remove this for some reason? Just curious :)

This comment has been minimized.

@stsewd

stsewd Feb 16, 2018

Member

@humitos the header is larger than the text p:

This comment has been minimized.

@humitos

humitos Feb 16, 2018

Member

Oh, that's OK. Github doesn't show it in a monospace font for some reason:

captura de pantalla_2018-02-16_09-18-14

This comment has been minimized.

@stsewd

stsewd Feb 16, 2018

Member

I use rstcheck for linting rst files :)

This comment has been minimized.

@humitos

humitos Feb 16, 2018

Member

Wow! I didn't know it and I'm a fan of @myint.

Considering that we are talking about the documentation style, I think it would be a good addition for travis checks and linting right? I mean, trigger a linting on the docs on travis checks.

Would you like to add an issue for this and take a look on what's needed and how to implement it?

This comment has been minimized.

@stsewd

stsewd Feb 16, 2018

Member

Sure, I opened a new issue #3622, I will implemented later :)

stsewd added some commits Feb 16, 2018

@stsewd

This comment has been minimized.

Member

stsewd commented Feb 16, 2018

@humitos all the corrections were done, thanks!

@humitos humitos requested a review from ericholscher Feb 16, 2018

@humitos

This comment has been minimized.

Member

humitos commented Feb 16, 2018

Nice! I'm asking Eric for review this since he is the "English guy". Although, it's good enough for me.

@ericholscher ericholscher merged commit f725917 into rtfd:master Feb 18, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ericholscher

This comment has been minimized.

Member

ericholscher commented Feb 18, 2018

Love doc updates :) 💯

@stsewd stsewd deleted the stsewd:update-docs branch Feb 18, 2018

@RichardLitt

This comment has been minimized.

Member

RichardLitt commented Feb 19, 2018

@humitos Feel free to ask me for English updates, too. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment