Skip to content

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