-
-
Notifications
You must be signed in to change notification settings - Fork 346
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
Update doxygen (cont'd) #1543
Update doxygen (cont'd) #1543
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1543 +/- ##
==========================================
- Coverage 70.47% 70.46% -0.02%
==========================================
Files 376 379 +3
Lines 59081 59080 -1
Branches 21222 21222
==========================================
- Hits 41640 41633 -7
- Misses 14360 14367 +7
+ Partials 3081 3080 -1
... and 14 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
f4195f6
to
da574fd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this, @ischoegl. I had just a few small comments.
//! Return a reference to the Jacobian evaluator of an OneDim object. | ||
//! @ingroup derivGroup |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see what you're trying to do by adding the class name to the docstring, but the way member functions are shown in Doxygen's "modules" leaves a fair bit to be desired, as I'm sure you've noticed. While I think the two groups touched on in this PR are okay, I'd suggest not going any further in the direction of trying to populate such groups, if you hadn't already come to that conclusion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to collect some spots where Jacobians are evaluated, as they are relevant to the new derivGroup
module. I indeed tried to mitigate the lacking presentation. I have no intention to make this more wide-spread.
There is some need to document PreconditionerBase
etc, which hopefully will take care of some of these stop-gap entries.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
breathe
may eventually provide us with an opportunity to adjust the presentation, so I think this is fine for now.
eac26ab
to
4354877
Compare
@speth ... I believe I addressed everything. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @ischoegl. This looks good to me.
Changes proposed in this pull request
oneD
documentationzeroD
documentationIf applicable, fill in the issue number this pull request is fixing
Resolves #1535
If applicable, provide an example illustrating new features this pull request is introducing
Checklist
scons build
&scons test
) and unit tests address code coverage