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

Documentation site rework #189

Merged
merged 36 commits into from
Dec 29, 2023
Merged

Documentation site rework #189

merged 36 commits into from
Dec 29, 2023

Conversation

eliotwrobson
Copy link
Collaborator

@eliotwrobson eliotwrobson commented Dec 3, 2023

Draft PR for the new documentation site. Docstrings have been added for all functions in the public interface, and the mkdocs site now generates the API from these docstrings.

@caleb531 still some polishing to go, including porting over examples, but there's enough here that it's probably worthwhile to start reviewing. In response to #160.

For reference, as we are submitting the package to PyOpenSci, here is their guide on writing package documentation: https://www.pyopensci.org/python-package-guide/documentation/write-user-documentation/get-started.html

For this review, we should make sure the new docs site specifically address all of the items in the guide.

@eliotwrobson eliotwrobson self-assigned this Dec 3, 2023
@coveralls
Copy link

coveralls commented Dec 3, 2023

Coverage Status

coverage: 99.64%. remained the same
when pulling c62d3ea on docs_rework
into fa2a6ec on develop.

@caleb531
Copy link
Owner

caleb531 commented Dec 4, 2023

@eliotwrobson Thanks for all your efforts to put this together! I won't be able to review today, but will probably take a look sometime during the week.

@eliotwrobson
Copy link
Collaborator Author

@caleb531 sounds great! I think most things from the old docs have been ported into the new site successfully. The main thing is making sure we have everything required to pass the review. It's OK to have things be a little barebones, since I think we will get some good suggestions for things to add to the docs from that review.

Once this is merged, I think the course of action will be to set up a publishing workflow, then merge to master to hopefully deploy the new site. Then, with the changes to some of the other repo files, we should be good to go with a full submission!

cc @lewiuberg since you did the initial work on this.

@eliotwrobson
Copy link
Collaborator Author

@caleb531 do you have some time to take a look at this soon? I don't think this needs a super detailed review since I know this is huge, but as long as it's at an MVP stage, we'll get additional comments from the PyOpenSci review that should iron out any remaining issues.

@caleb531
Copy link
Owner

@eliotwrobson Yep I can take a look tonight, but if it's ready for my review, shouldn't we take this PR out of Draft Mode?

@eliotwrobson
Copy link
Collaborator Author

@caleb531 yep! Good call, I forgot to take this out of draft when I added the rest of the documentation.

@eliotwrobson eliotwrobson marked this pull request as ready for review December 28, 2023 17:21
@caleb531
Copy link
Owner

caleb531 commented Dec 29, 2023

@eliotwrobson Reviewing this now! Can you please remind me how I can run the documentation site locally? Do we have that documented somewhere (perhaps in CONTRIBUTING.md in this PR)?

@eliotwrobson
Copy link
Collaborator Author

eliotwrobson commented Dec 29, 2023

@caleb531 the commands are in #160, last three commands here: #160 (comment)

@caleb531
Copy link
Owner

@eliotwrobson Oh yes that's right, I have actually run this before 😅. Thank you! Reviewing now...

Copy link
Owner

@caleb531 caleb531 left a comment

Choose a reason for hiding this comment

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

@eliotwrobson I've finished my review! I tried to keep my comments few and focused. I think the biggest thing would be solving the 404 when you land at the root of the documentation site, but as I've found, that should be easily fixable.

Thank you for all the time you've put into this (and you too, @lewiuberg)!

docs/introduction.md Outdated Show resolved Hide resolved
docs/introduction.md Outdated Show resolved Hide resolved
mkdocs.yml Outdated Show resolved Hide resolved
docs/assets/img/logo.svg Outdated Show resolved Hide resolved
@eliotwrobson
Copy link
Collaborator Author

@caleb531 addressed all of your comments! If it looks good and you approve, I can go ahead and merge, and then I'll try and work on a deployment workflow as part of a followup PR.

@caleb531
Copy link
Owner

@eliotwrobson So I guess I was looking at a cached version of the docs site, because I just hard-refreshed, and the theme is way better. The sidebar is also always-visible, which I remember specifically requesting.

All that to say, this docs site is looking really great, and I am totally onboard now. I think the search functionality will be much appreciated as well.

Screenshot 2023-12-29 at 10 06 28 AM

Copy link
Owner

@caleb531 caleb531 left a comment

Choose a reason for hiding this comment

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

@eliotwrobson Okay, all conversations are resolved, so I'm ready to move this forward. Approved!

Please feel free to merge when you are ready.

@eliotwrobson
Copy link
Collaborator Author

@caleb531 Awesome! I'll merge this now and get to work on the workflow later this afternoon.

@eliotwrobson eliotwrobson merged commit 3444b17 into develop Dec 29, 2023
14 checks passed
@caleb531 caleb531 mentioned this pull request Dec 31, 2023
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