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

Add documentation #62

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Add documentation #62

wants to merge 4 commits into from

Conversation

dbrgn
Copy link
Member

@dbrgn dbrgn commented Sep 19, 2017

Argh, I only saw your docs branch once I pushed mine.

Anyways, here's documentation implemented using mkdocs, which is a bit simpler and faster than sphinx. It does not support generating API docs, but I think API docs are overrated anyways and prose is much more important.

If you like this I can set up auto builds with Travis on Readthedocs.

Refs #2

@dbrgn dbrgn requested a review from lgrahl September 19, 2017 07:51
@dbrgn
Copy link
Member Author

dbrgn commented Sep 19, 2017

To build: Install the requirements.txt and run make.

@codecov-io
Copy link

codecov-io commented Sep 19, 2017

Codecov Report

Merging #62 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #62   +/-   ##
=======================================
  Coverage   76.76%   76.76%           
=======================================
  Files           9        9           
  Lines        1278     1278           
  Branches      153      153           
=======================================
  Hits          981      981           
  Misses        250      250           
  Partials       47       47

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9b2a2ee...48b7247. Read the comment docs.

@lgrahl
Copy link
Member

lgrahl commented Sep 19, 2017

Mh... I mean we have no real API doc so far anyway... I'll need to think about this. Would definitely like to migrate to Sphinx later (converting the Markdown files is easy).

@dbrgn
Copy link
Member Author

dbrgn commented Sep 19, 2017

That'll be easy, but it's better to have non-sphinx docs than to have no docs.

recommend using [venv](https://docs.python.org/3/library/venv.html) to create
an isolated Python environment.

$ python3 -m venv venv
Copy link
Member

Choose a reason for hiding this comment

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

While you're at it, can you patch this in the readme, too?

Copy link
Member

Choose a reason for hiding this comment

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

^

during the next 5 years:

$ openssl req \
-newkey rsa:1024 \
Copy link
Member

Choose a reason for hiding this comment

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

I'd say 4096. Sure, it's a test certificate but you know... people do stupid things and I don't want to be responsible. 😉

Copy link
Member Author

Choose a reason for hiding this comment

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

The certificate is only valid for localhost. I don't think there's much harm that can be caused by that. Do you think there's any value in >1024?

Copy link
Member

Choose a reason for hiding this comment

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

I was thinking of people that generate production certs and are looking for examples.

-config <(cat /etc/ssl/openssl.cnf \
<(printf '[SAN]\nsubjectAltName=DNS:localhost')) \
-sha256 \
-days 1825
Copy link
Member

Choose a reason for hiding this comment

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

Capping to 90 days should be sufficient. One doesn't want a test cert to be valid for a long period of time if it's meant for short-term testing. In case one forgets to delete it from Chrome, it can't do any harm (for long).

Copy link
Member Author

Choose a reason for hiding this comment

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

To be honest it's a huge PITA having to generate a new test-cert and having to import it both into Firefox and Chrome every time I want to run SaltyRTC tests (which is often more than 90 days apart).

Copy link
Member

Choose a reason for hiding this comment

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

Yes but you are an intelligent being that can adjust this parameter. 😉

docs/mkdocs.yml Outdated
@@ -0,0 +1,21 @@
site_name: SaltyRTC Server
site_description: Documentation for saltyrtc-server-python.
site_author: Lennart Grahl
Copy link
Member

Choose a reason for hiding this comment

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

Feel free to add yourself.

@dbrgn
Copy link
Member Author

dbrgn commented Sep 20, 2017

Updated!

Copy link
Member

@lgrahl lgrahl left a comment

Choose a reason for hiding this comment

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

Sorry. :)

recommend using [venv](https://docs.python.org/3/library/venv.html) to create
an isolated Python environment.

$ python3 -m venv venv
Copy link
Member

Choose a reason for hiding this comment

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

^

@dbrgn dbrgn self-assigned this Sep 21, 2017
@dbrgn
Copy link
Member Author

dbrgn commented Sep 22, 2017

Actually I'd link to the docs from the README file. There's no advantage in keeping the information in two places and always having to sync the docs and the README.

But for that I'd have to set up automated test building etc. Should I?

@lgrahl
Copy link
Member

lgrahl commented Sep 24, 2017

But for that I'd have to set up automated test building etc.

What do you mean by that?

@dbrgn
Copy link
Member Author

dbrgn commented Sep 24, 2017

Setting up automated builds of the docs through Readthedocs, triggered by successful tests :) Once we have always-up-to-date docs, we can link to them from the README and remove the instructions from the README.

@dbrgn
Copy link
Member Author

dbrgn commented Jul 30, 2019

@lgrahl still interested in this?

@lgrahl
Copy link
Member

lgrahl commented Jul 30, 2019

I think it's a good reminder that we need docs and this is a good start. Let's leave it open.

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.

4 participants