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 back missing docs #196

Merged
merged 1 commit into from
Dec 30, 2023

Conversation

eliotwrobson
Copy link
Collaborator

Added back missing docs page from move to mkdocs. Uses change from #194

Co-authored-by: anshravalll <124378264+anshravalll@users.noreply.github.com>
@eliotwrobson eliotwrobson self-assigned this Dec 30, 2023
@coveralls
Copy link

Coverage Status

coverage: 99.64%. remained the same
when pulling 6e3090a on eliotwrobson:add_back_missing_docs
into 3444b17 on caleb531:develop.

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.

This looks good! But quick question, and something I've been wondering:

I suppose I was under the impression that we'd be converting all of the Markdown pages to docstrings within the code. But it seems like we'll be keeping both? Can you please clarify where Markdown is being replaced by docstrings and where it isn't?

@eliotwrobson
Copy link
Collaborator Author

@caleb531 to your question, I think it makes sense to at least keep all of the API documentation in docstrings and have that generated by mkdocs. Putting everything in a docstring doesn't make sense for some of the documentation, especially in the case of long examples or extended discussions, such as the automaton characteristics here. However, I think this is the only case where there was some extended discussion in the old API documentation that didn't really fit into a docstring.

In this case, I think the change is more organizational than anything else (as the new docs page describes general behavior rather than specific methods), and it's questionable whether this discussion really should have ever been included as part of the API, rather than as part of a more user-facing quick-start guide.

I think this is a good question though, since we will probably add more documentation based on feedback from the upcoming review. Going to go ahead and merge, and if the organization of this page seems confusing we can discuss moving it somewhere else.

@eliotwrobson eliotwrobson merged commit f9189d1 into caleb531:develop Dec 30, 2023
7 checks passed
@eliotwrobson eliotwrobson deleted the add_back_missing_docs branch December 30, 2023 18:50
@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.

3 participants