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 improvements to improve documentation of C API #2355

Merged

Conversation

ChuckHastings
Copy link
Collaborator

Some doxygen improvements:

  • Defined a C API module
  • Defined submodules for each C API algorithm header
  • Moved sampling functions into a separate header
  • Moved traversal functions into a separate header
  • Deleted a few old nvgraph doxygen things that were interfering with a clean definition of the cugraph namespace

@ChuckHastings ChuckHastings self-assigned this Jun 15, 2022
@ChuckHastings ChuckHastings added 2 - In Progress doc Documentation improvement Improvement / enhancement to an existing function non-breaking Non-breaking change and removed improvement Improvement / enhancement to an existing function labels Jun 15, 2022
@ChuckHastings ChuckHastings added this to the 22.08 milestone Jun 15, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jun 16, 2022

Codecov Report

❗ No coverage uploaded for pull request base (branch-22.08@95824f8). Click here to learn what that means.
The diff coverage is n/a.

@@               Coverage Diff               @@
##             branch-22.08    #2355   +/-   ##
===============================================
  Coverage                ?   60.74%           
===============================================
  Files                   ?      105           
  Lines                   ?     5075           
  Branches                ?        0           
===============================================
  Hits                    ?     3083           
  Misses                  ?     1992           
  Partials                ?        0           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 95824f8...4f0442a. Read the comment docs.

@ChuckHastings ChuckHastings marked this pull request as ready for review June 29, 2022 19:00
@ChuckHastings ChuckHastings requested review from a team as code owners June 29, 2022 19:00
@@ -62,7 +62,7 @@ for FILE in conda/environments/*.yml; do
done

# Doxyfile update
sed_runner "s|\(TAGFILES.*librmm/\).*|\1${NEXT_SHORT_TAG}|" cpp/doxygen/Doxyfile
sed_runner "s|PROJECT_NUMBER\s*=\s*${CURRENT_SHORT_TAG}|PROJECT_NUMBER=${NEXT_SHORT_TAG}|" cpp/doxygen/Doxyfile
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
sed_runner "s|PROJECT_NUMBER\s*=\s*${CURRENT_SHORT_TAG}|PROJECT_NUMBER=${NEXT_SHORT_TAG}|" cpp/doxygen/Doxyfile
sed_runner "s|PROJECT_NUMBER\s*=.*|PROJECT_NUMBER=${NEXT_SHORT_TAG}|" cpp/doxygen/Doxyfile

I think it's a good practice to make the first pattern in these sed expression not depend on any particular version (i.e. ${CURRENT_SHORT_TAG}) because this will prevent the expression from properly replacing values when they're out of sync (which seems like the case here since PROJECT_NUMBER is many versions behind at 0.14).

Disclaimer: I didn't test this expression change locally.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated. I also tested the script locally and fixed an issue in my regex.

@ChuckHastings
Copy link
Collaborator Author

rerun tests

@BradReesWork
Copy link
Member

@gpucibot merge

@rapids-bot rapids-bot bot merged commit d4e8e2d into rapidsai:branch-22.08 Jul 5, 2022
@ChuckHastings ChuckHastings deleted the update_doxygen_for_c_api branch August 4, 2022 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Documentation non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants