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

Update doxygen (cont'd) #1543

Merged
merged 9 commits into from Jul 18, 2023
Merged

Update doxygen (cont'd) #1543

merged 9 commits into from Jul 18, 2023

Conversation

ischoegl
Copy link
Member

@ischoegl ischoegl commented Jul 17, 2023

Changes proposed in this pull request

  • Restructure doxygen File Input/Output documentation
  • Restructure oneD documentation
  • Restructure zeroD documentation
  • Add tree view

If 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

  • The pull request includes a clear description of this code change
  • Commit messages have short titles and reference relevant issues
  • Build passes (scons build & scons test) and unit tests address code coverage
  • Style & formatting of contributed code follows contributing guidelines
  • The pull request is ready for review

@codecov
Copy link

codecov bot commented Jul 17, 2023

Codecov Report

Merging #1543 (4354877) into main (25e8e5c) will decrease coverage by 0.02%.
The diff coverage is n/a.

@@            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     
Impacted Files Coverage Δ
include/cantera/base/AnyMap.h 80.00% <ø> (ø)
include/cantera/base/Interface.h 100.00% <ø> (ø)
include/cantera/base/Solution.h 87.50% <ø> (ø)
include/cantera/base/SolutionArray.h 94.44% <ø> (ø)
include/cantera/base/Units.h 90.90% <ø> (ø)
include/cantera/base/YamlWriter.h 100.00% <ø> (ø)
include/cantera/base/ctexceptions.h 60.86% <ø> (ø)
include/cantera/base/global.h 81.81% <ø> (ø)
include/cantera/kinetics/EdgeKinetics.h 100.00% <ø> (ø)
include/cantera/kinetics/ImplicitSurfChem.h 100.00% <ø> (ø)
... and 32 more

... and 14 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ischoegl ischoegl marked this pull request as ready for review July 17, 2023 19:21
@ischoegl ischoegl requested a review from a team July 17, 2023 19:21
Copy link
Member

@speth speth left a 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.

include/cantera/oneD/Boundary1D.h Outdated Show resolved Hide resolved
Comment on lines +48 to +49
//! Return a reference to the Jacobian evaluator of an OneDim object.
//! @ingroup derivGroup
Copy link
Member

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.

Copy link
Member Author

@ischoegl ischoegl Jul 17, 2023

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.

Copy link
Member

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.

include/cantera/oneD/StFlow.h Outdated Show resolved Hide resolved
include/cantera/kinetics/solveSP.h Outdated Show resolved Hide resolved
@ischoegl
Copy link
Member Author

@speth ... I believe I addressed everything.

Copy link
Member

@speth speth left a 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.

@ischoegl ischoegl merged commit 037d958 into Cantera:main Jul 18, 2023
33 of 40 checks passed
@ischoegl ischoegl deleted the update-doxygen2 branch July 18, 2023 00:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Missing doxygen groupings
2 participants