-
Notifications
You must be signed in to change notification settings - Fork 95
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 #208
Comments
@edwardhartnett Your proposal sounds good to me. Please proceed your PR. |
@edwardhartnett - I am absolutely on board with this. In fact, this is on the list of DTC potential contributions for this year depending on resource availability. I have only dabbled with doxygen, so to have some expertise really help us get moving forward quickly would be very welcome. One important aspect from my viewpoint was to try to establish an agreed upon set of standards rom the start, and it sounds like NCEPLIBS has already taken the reign on that which will be helpful. How smooth is the converting process? There are a lot of outdated comments in the code, commented code blocks, etc. Is it cumbersome to modify/correct doxygenated comments on the fly as needed? Or would you anticipate a rather thorough review needed after first attempt? One concern is the timing of this initial implementation. With phase 1 of the refactored code just around the corner, does it make sense to wait until after that is merged into develop? Would this be best suited for moratorium? Related to the timing, the other concern I have is resource availability to ensure this feature is well documented and supported for all developers, and if this comes from DTC as originally discussed, @WenMeng-NOAA @HuiyaChuang-NOAA and I should have a discussion about roles and responsibilities and timelines. |
@fossell and @HuiyaChuang-NOAA I agree with Kate that this PR would be merged after refactor phase 1. I would expect the refactor phase 1 merge will be done within 3 weeks. With the doxygen PR from @edwardhartnett on submitted, all of us might have more discussions before it is finally incorporated in the UPP repos. |
I whole heartedly support documentation with doxygen. There is an overarching document/guide in rst format that is accessed through readthedocs, but getting the code documented in doxygen will be very good. I think this will have to be an ongoing process. We will need SMEs on board, once the initial framework is in place. What is the time line for the first merge back in? modifying / correcting documentation should be like any other code review process, but probably requires a closer look from all of you to verify the veracity of the documentation. Moratorium for us has more or less begun as codes have been delivered. This is the time to clean up because we will soon be busy with transition to WCOSS2 activities |
Each code file will change. The comments at the top of the file and in front of each subprogram must change. Take a look here for examples: ufs-community/UFS_UTILS#198 So if you are working on some big refactor, and about to merge to develop, then I will wait. Let me know when it is a good time. I will do a first pass and convert your existing comments. Doxygen is pretty trivial, so programmers can just cut and paste and learn by example. There are endless resources on the web. There is a method of converting the doxygen output further into .rst, so you can add this to your readthedocs documentation, if you want it to contain this level of detail. I am using the GitHub gh-pages to put the documentation up on the web for each repo. It's easy, but I can't do it for this repo because I can't write branches to it. I can explain how to do it if you like. |
@edwardhartnett Thanks for pointing us the example. It appears to me the doxygen heads will be added all existing routines. We will have a big merge from the UPP refactor phase 1 which have changes in more than 35 routines. I would expect it will be done |
Volbona had started on this effort in April and told me she's done some work. Should we leverage off that? |
I would be interested in hearing about any previous work on this, of course. |
@edwardhartnett The UPP refactor phase 1 merge was just made in the branch develop. You might start your doxygen work. Let me know if you have any questions. Thanks! |
Yes. That’s the plan. I’ve asked that Bo and Jesse to finish their next phase of merge
By our Nov 30 tag-up. We will also discuss the plan for code reviews at this
Meeting.
Huiya
…Sent from my iPad
On Oct 29, 2020, at 9:30 AM, WenMeng-NOAA ***@***.***> wrote:
@fossell and @HuiyaChuang-NOAA I agree with Kate that this PR would be merged after refactor phase 1. I would expect the refactor phase 1 merge will be done within 3 weeks. With the doxygen PR from @edwardhartnett on submitted, all of us might have more discussions before it is finally incorporated in the UPP repos.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
@edwardhartnett Have all changes been made in the UPP? If yes, may I close this issue? |
No, there is more to do. I was waiting for some outstanding PRs to be merged, and now they are. I will do a PR with another round of doxygen changes next week... |
@edwardhartnett Your plan works to me. Thanks! |
I have been pulled away and will not be able to return to this documentation task. I hope I have helped you get started on your doxygen journey. On the UFS_UTILS project, we now have full doxygen documentation on every function and subroutine. This allows us to turn on automatic checking of new code. Now it is no longer possible for programmers to merge undocumented code. This checking is very helpful! I would urge you to run doxygen with warnings on, and then fix all warnings. When you are running without warning, turn on the option WARN_AS_ERROR, and you will any undocumented code will break the build. Let me know if you need help setting this up. |
@edwardhartnett Thanks for helping us initially setting up doxygen documentation. |
Add the fix/ directory to regional_workflow - GSI fix files under fix/gsi - UPP fix files under fix/upp - CRTM fix files under fix/crtm - change scripts, config.sh*, etc accordingly Add Init.sh - initialize fix/ so that links to binary/large fix files work at different HPC platforms - install pre-commit git hook to prevent binary/large fix files themselves to be committed Add fix_rrfs_locations.sh - list the FIX_RRFS locations at differnt HPC platforms AIRCRAFT_REJECT, SFCOBS_USELIST - default to use fix files under FIX_GSI unless a path specified in config.sh Add $TESTBED_FIELDS_FN (files are under FIX_UPP) - different testbed_fields files for different applications - if empty, no bgsfc output
The NCEPLIBS team is adding doxygen based documentation to all NCEPLIBS libraries. I believe EMC_post should also add doxygen.
Doxygen is the world leader in generating documentation from code, used by many of the libraries we rely on, including netCDF and HDF5. It is widely understood and supported on all platforms.
To see the kind of documentation we have generated so far, take a look here: https://noaa-emc.github.io/NCEPLIBS/
The documentation for NCEPLIBS is still in first draft form, but here's a project with a reasonably complete example: https://noaa-emc.github.io/NCEPLIBS-sp/
If it is agreed, then the first step will be to add the docs directory, the Doxyfile.in file, and the CMake support for building doxygen docs. Then would come a series of PRs in which existing Fortran files doxygenated, converting exisisting comments into doxygen comments. For a good example of what this looks like, see this open PR: ufs-community/UFS_UTILS#198
Note that this means all programmers on EMC_post would have to respect and maintain the doxygen documentation. It is not hard to do, but I'm not signing up to be your tech writer - I have important coding to do as well. ;-) But I will get you started if you like.
The format of the doxygen comments should roughly match the rest of the NCEPLIBS documentation, since they will all be the same documentation set for the users. There is a long discussion of doxygen and how it may be used on our codes here: NOAA-EMC/NCEPLIBS#121
The text was updated successfully, but these errors were encountered: