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

Adding documentation (Sphinx) and docstrings #164

Merged

Conversation

kallewesterling
Copy link
Collaborator

@kallewesterling kallewesterling commented Jun 13, 2024

This PR includes a lot of edits:

  • Moving of the old documentation to a separate folder (then entirely removed)
  • Adding in Sphinx + theme (sphinx-book-theme) and plug-ins needed (described in docs/requirements.txt
  • Adding all the documentation from former docs folder (markdown) into new format (RST) in the new docs directory
  • Adding in docstrings (or placeholders for some docstrings) for most methods and functions across the codebase.

@edwardchalstrey1
Copy link
Owner

Thanks @kallewesterling this looks great! I'll have a proper review at some point next week I think. In the meantime I'll use today's Seshat meeting to check on Majid's progress with merging the work on this fork so far to the upstream repo and updating the production site. Until that's done I've been avoiding editing parts of the code that aren't directly related to the maps, but I'm hopeful we'll be at that point very soon and can do things like this updating his docstrings etc and do more regular PRs to avoid large conflicts

@edwardchalstrey1
Copy link
Owner

edwardchalstrey1 commented Jun 25, 2024

Hi @kallewesterling I've just had a go at checking this out locally and building the docs (and aim to look at the conflicts next). I've added sphinx-rtd-theme which seems to be required: kallewesterling#1 (in future feel free to use my repo instead of your fork when making branches/PRs)

On building the docs I'm currently getting these errors (and it seems to be either hanging now or just taking a very long time) - this all comes after [AutoAPI] Reading files... [100%] Update: this was caused by me having a venv for Pulumi in the directory structure

@edwardchalstrey1
Copy link
Owner

edwardchalstrey1 commented Jun 25, 2024

Actually perhaps I should try setting up a new python env specifically for docs as the error seems to be caused by Pulumi A new Python env was not required

@edwardchalstrey1
Copy link
Owner

Ok nice, docs building for me now - will have a proper review tomorrow 👍

@kallewesterling
Copy link
Collaborator Author

Great to hear!! Let me know if you run into any other problems!

@edwardchalstrey1
Copy link
Owner

I have opened a new issue #175 to ensure tests can be run by actions in future, but for now I have confirmed tests run locally so will merge

@edwardchalstrey1 edwardchalstrey1 merged commit 15d7cfe into edwardchalstrey1:dev Jun 26, 2024
1 check failed
@kallewesterling kallewesterling deleted the adding-docstrings branch July 5, 2024 11:12
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