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

Doxygen documentation for lib/substitutions #1339

Draft
wants to merge 45 commits into
base: master
Choose a base branch
from

Conversation

Bob-Chen222
Copy link
Contributor

@Bob-Chen222 Bob-Chen222 commented Mar 28, 2024

Description of changes:
Created documentation for core functions and structs in the substitution library lib/substitutions. Previous PR closed by accident; created a new one.

For reference, previous PR: #1309

Change made on April 3rd: add tutorial markdown file in the substitution folder

Related Issues:

Linked Issues:

Issues closed by this PR:


This change is Reviewable

@lockshaw lockshaw changed the title Repo refactor Doxygen documentation for lib/substitutions Mar 28, 2024
@lockshaw lockshaw requested review from lockshaw and wmdi March 28, 2024 05:16
@lockshaw
Copy link
Collaborator

The high level style here looks pretty good. My only general concern is that some of the docstrings are on the long side--there are many cases in which this is fine (and the explanations generally look pretty good), but remember that all documentation we write now has to be maintained throughout code changes, so while it is important that docstrings include details that help users understand the code beyond the function names and prototypes, it's equally important that they be as short and focused as they can while still being clear. It would be a good idea for you to do a quick pass through and see if there are any parts of the docstrings that you feel aren't critical and can be removed without losing much--not saying there are (as I haven't had time to do a super in-depth read), but it's worth an additional check.

I'm handing this PR off to @wmdi as she's better equipped to do fine-grained checking that everything is correct as she's more familiar with this code. Of course feel free to tag me in if there are any questions on the overall style/structure of the documentation 🙂

@lockshaw lockshaw removed their request for review March 28, 2024 06:04
@lockshaw
Copy link
Collaborator

Currently waiting on #1394 to get merged, and then needs to be updated with the new substitutions changes there

@lockshaw lockshaw marked this pull request as draft May 31, 2024 05:48
@lockshaw lockshaw changed the base branch from repo-refactor to master December 16, 2024 08:39
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