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

Add Doxygen documentation for H5FD module and h5tools. #3909

Closed
wants to merge 2 commits into from

Conversation

hyoklee
Copy link
Member

@hyoklee hyoklee commented Dec 23, 2023

This will help tool users by exposing h5dump VFD options.

20231222_h5tools_doxygen

20231222_h5tools_doxygen_drivernames

Once this PR is merged, add the link to https://portal.hdfgroup.org/documentation/hdf5-docs/registered_virtual_file_drivers_vfds.html for #3878.

P.S. H5FD was missing so I added it as well.

Copy link
Contributor

@byrnHDF byrnHDF left a comment

Choose a reason for hiding this comment

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

This basically exposes the tools API, which we have avoided because of a lack of design and resources. This "publish the API" needs a company directive.
In addition this adds a "C" source file to the doxygen. Also, we should not add just the C library APIs for FD package without the adding the other languages.

@lrknox
Copy link
Collaborator

lrknox commented Dec 26, 2023

This basically exposes the tools API, which we have avoided because of a lack of design and resources. This "publish the API" needs a company directive.
@derobins What do you think?

@bmribler bmribler added Component - Documentation Doxygen, markdown, etc. Type - Improvement Improvements that don't add a new feature or functionality labels Dec 26, 2023
@jhendersonHDF
Copy link
Collaborator

I think we definitely need something like this, but also think it should just be a table in the H5FD module file doxygen. I don't think it makes sense to tie it to the tools, especially since these changes are just exposing an implementation detail.

@derobins
Copy link
Member

We should expose the names, but not in this way. I'm going to go ahead and close this.

@hyoklee - Please create an issue for this and add it to the 1.14.4 milestone.

@derobins derobins closed this Jan 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component - Documentation Doxygen, markdown, etc. Type - Improvement Improvements that don't add a new feature or functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants