doxygenated fv3_cap.F90 #819
Conversation
There was a problem hiding this comment.
Looks good!
Comment changes only in this PR, no code changes.
@junwang-noaa or @DusanJovic-NOAA can you help by telling us what the "???" should be?
| module fv_moving_nest_main_mod | ||
|
|
||
| #include <fms_platform.h> |
There was a problem hiding this comment.
Why are there so many code changes in this file?
There was a problem hiding this comment.
I'm trying to investigate why it's showing up this way. I double checked this file in your repo and compared it to develop. It showed zero code changes, but after swapping the files for some reason it's showing up as huge chunks of changes. I followed the same procedure as the other 2 files.
There was a problem hiding this comment.
Not sure what happened, but it's fixed now.
There was a problem hiding this comment.
Much better, thanks!
| !> ??? | ||
| type(ESMF_GridComp) :: fcstComp | ||
|
|
||
| !> ??? | ||
| type(ESMF_State) :: fcstState | ||
|
|
||
| !> ??? | ||
| type(ESMF_FieldBundle), allocatable :: fcstFB(:) | ||
|
|
||
| !> ??? | ||
| integer,dimension(:), allocatable :: fcstPetList | ||
|
|
||
| !> ??? |
There was a problem hiding this comment.
All these variables are local (private) variables and are not accessible outside the module. Are you going to add doxygen the every local variable in every module and subroutine? I thought we are adding doxygen just to subroutine arguments and public module variables.
There was a problem hiding this comment.
We will not be adding doxygen for any subprogram variables.
Good question about private module variables, I will investigate and get back to you. My hunch is that doxygen doesn't distinguish between different types of module variables.
We need to document all the module variables that doxygen will complain about if undocumented. This is so we can turn on warning-check and ensure that no undocumented code is committed to the repo.
|
@AlexanderRichert-NOAA can you get the fv3atm CI working again? |
|
@DusanJovic-NOAA what's the status of the mpi_f08 issue, did you work out a fix? |
|
@AlexanderRichert-NOAA Yes, mpi_f08 will be fixed in #805, which is merged with #811 which will be merged soon. But unfortunately there was an update in upp which is causing workflow to fail now at: sed -i 's/doc /upp_doc /' upp/docs/CMakeLists.txt So, we'll need one more update. Do you want to fix this (remove that sed command) in your 'Add unit testing' PR? |
|
It's not a rush, if a fix is coming we can wait for it... I just wanted to get the CI working again so we can start running doxygen in it. |
|
I would recommend fixing the sed failure in #811, just so we don't keep merging PRs where the CI isn't working. That said, I agree it should be as simple as removing that line since the custom target in UPP now has the name 'upp_doc' already. |
|
ufs-wm code managers already started or actually finished regression testing, so that change must go in as part of the next PR. |
|
@AlexanderRichert-NOAA @edwardhartnett I removed that sed command from fv3atm workflow file that renames 'doc' to 'upp_doc' in the UPP's CMakeLists.txt but the workflow is still failing: https://github.com/NOAA-EMC/fv3atm/actions/runs/8833224366/job/24252100176#step:6:220 There is a mismatch in the name of doc folders in UPP version fv3atm is using. I see that is fixed 3 days ago in UPP https://github.com/NOAA-EMC/UPP/pull/932/files @WenMeng-NOAA Looks like we will need another fv3atm PR that updates UPP submodule. |
Description
More doxygen
Issue(s) addressed
Part of #760
Testing
Documentation changes only
Dependencies
N/A