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

fix the documentation build for fv3atm, getting the CI to work #906

Merged
merged 12 commits into from
Jan 21, 2025

Conversation

edwardhartnett
Copy link
Collaborator

@edwardhartnett edwardhartnett commented Jan 9, 2025

Description

In this PR I change the CMake option ENABLE_DOCS to ENABLE_FV3ATM_DOCS, in order to avoid invoking the documentation build of UPP.

I also add a new CI workflow called "docs", and take the documentation build out of the GCC workflow. This will be more useful to programmers because having the "docs" CI run fail is pretty obvious.

There are no code changes in this PR; this is documentation and CI only.

Issue(s) addressed

Fixes #904

Testing

CI testing. No code changes in this PR.

Dependencies

N/A

Requirements before merging

  • All new code in this PR is tested by at least one unit test
  • All new code in this PR includes Doxygen documentation
  • All new code in this PR does not add new compilation warnings (check CI output)

@edwardhartnett edwardhartnett marked this pull request as ready for review January 9, 2025 19:04
@edwardhartnett edwardhartnett changed the title fix the documentation build for fv3atm fix the documentation build for fv3atm, getting the CI to work Jan 9, 2025
This was referenced Jan 9, 2025
Copy link
Collaborator

@DusanJovic-NOAA DusanJovic-NOAA left a comment

Choose a reason for hiding this comment

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

I am not familiar with Doxygen and the changes required in the workflow to support its use, so I can not properly review this PR. I will approve it, assuming the code here does what it's supposed to do.

@edwardhartnett
Copy link
Collaborator Author

Who is the other reviewer required to make this PR mergable?

@DusanJovic-NOAA
Copy link
Collaborator

Who is the other reviewer required to make this PR mergable?

@junwang-noaa

@edwardhartnett
Copy link
Collaborator Author

Probably we need to add @LarissaReames-NOAA to the list of those who can approve a PR here.

@junwang-noaa can you please take a look at this PR and approve it?

@junwang-noaa
Copy link
Collaborator

junwang-noaa commented Jan 10, 2025

@edwardhartnett I approved it already, please see the comments above. After @LarissaReames-NOAA accept the invitation, I will add her to the list. Thanks

Probably we need to add @LarissaReames-NOAA to the list of those who can approve a PR here.

@junwang-noaa can you please take a look at this PR and approve it?

@edwardhartnett
Copy link
Collaborator Author

Thanks!

@edwardhartnett
Copy link
Collaborator Author

Can this be merged please?

@edwardhartnett
Copy link
Collaborator Author

@DusanJovic-NOAA @LarissaReames-NOAA can this be merged please?

@DusanJovic-NOAA
Copy link
Collaborator

@LarissaReames-NOAA Should we merge this PR directly, without running standard ufs-weather-model tests and update the FV3 submodule pointer in ufs-wm, as we (currently) do with other fv3atm PRs?

@LarissaReames-NOAA
Copy link
Collaborator

@DusanJovic-NOAA If I'm understanding this PR correctly, it presents a unique case where no UWM tests could be affected since it's all GitHub and FV3-specific CI code changes unrelated to UWM tests or forecasts. I've never merged a PR here myself, but if you think that's the best way to handle this, I'm on board.

@DusanJovic-NOAA DusanJovic-NOAA merged commit 6b0794d into NOAA-EMC:develop Jan 21, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

doxygen build currently broken in CI runs
5 participants