Skip to content

Conversation

@WalterKolczynski-NOAA
Copy link
Contributor

@WalterKolczynski-NOAA WalterKolczynski-NOAA commented Dec 3, 2025

Description

Updates the ush scripts (including dev/ush) to be compliant with the static bash code analysis checkers.

Also updates the reporter for the GH action so that it should be able to post recommendations on PRs.

Type of change

  • Maintenance (code refactor, clean-up, new CI test, etc.)

Change characteristics

  • Is this a breaking change (a change in existing functionality)? NO
  • Does this change require a documentation update? NO
  • Does this change require an update to any of the following submodules? NO

How has this been tested?

  • All the PR-level tests in the cases directory on Ursa

Checklist

  • Any dependent changes have been merged and published
  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have documented my code, including function, input, and output descriptions
  • My changes generate no new warnings
  • New and existing tests pass with my changes
  • This change is covered by an existing CI test or a new one has been added
  • Any new scripts have been added to the .github/CODEOWNERS file with owners
  • I have made corresponding changes to the system documentation if necessary

Copy link
Contributor

@DavidHuber-NOAA DavidHuber-NOAA left a comment

Choose a reason for hiding this comment

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

Looks good overall. A few suggestions and questions.

Comment on lines +77 to 84
"${EXECgfs}/${pgm}" 1> "prnc_${WAVECUR_FID}_${ymdh_rtofs}.out" 2>&1
export err=$?
err_chk
if [[ "${err}" -ne 0 ]]; then
cat "prnc_${WAVECUR_FID}_${ymdh_rtofs}.out"
echo "WARNING: NON-FATAL ERROR IN ${pgm}."
exit 4
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

@JessicaMeixner-NOAA Should this be a fatal error or not? err_chk is going to exit completely if err is not 0, so the WARNING will never be printed.

Copy link
Contributor

@DavidHuber-NOAA DavidHuber-NOAA left a comment

Choose a reason for hiding this comment

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

Just one addition needed to retain noaacloud support.

module use "${HOMEgfs}/sorc/gdas.cd/modulefiles"

case "${MACHINE_ID}" in
"hera" | "orion" | "hercules" | "wcoss2" | "gaeac5" | "gaeac6" | "ursa")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"hera" | "orion" | "hercules" | "wcoss2" | "gaeac5" | "gaeac6" | "ursa")
"hera" | "orion" | "hercules" | "wcoss2" | "gaeac5" | "gaeac6" | "ursa" | "noaacloud")

@DavidHuber-NOAA
Copy link
Contributor

Launching CI tests.

@emcbot emcbot added CI-Gaeac6-Ready **CM use only** PR is ready for CI testing on Gaea C6 CI-Hercules-Ready **CM use only** PR is ready for CI testing on Hercules CI-Ursa-Ready **CM use only** PR is ready for CI testing on Ursa labels Dec 9, 2025
@DavidHuber-NOAA DavidHuber-NOAA added the CI-Wcoss2-Running CI testing on WCOSS for this PR is in-progress label Dec 9, 2025
@emcbot emcbot added CI-Hercules-Building **Bot use only** CI testing is cloning/building on Hercules CI-Ursa-Building **Bot use only** CI testing is cloning/building on Ursa CI-Gaeac6-Building **Bot use only** CI testing is cloning/building on Gaea C6 and removed CI-Hercules-Ready **CM use only** PR is ready for CI testing on Hercules CI-Ursa-Ready **CM use only** PR is ready for CI testing on Ursa CI-Gaeac6-Ready **CM use only** PR is ready for CI testing on Gaea C6 labels Dec 9, 2025
@emcbot emcbot added CI-Ursa-Running **Bot use only** CI testing on Ursa for this PR is in-progress CI-Hercules-Running **Bot use only** CI testing on Hercules for this PR is in-progress CI-Gaeac6-Running **Bot use only** CI testing on Gaea C6 for this PR is in-progress and removed CI-Ursa-Building **Bot use only** CI testing is cloning/building on Ursa CI-Hercules-Building **Bot use only** CI testing is cloning/building on Hercules CI-Gaeac6-Building **Bot use only** CI testing is cloning/building on Gaea C6 labels Dec 9, 2025
Copy link
Contributor

@DavidHuber-NOAA DavidHuber-NOAA left a comment

Choose a reason for hiding this comment

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

Approve pending successful CI testing.

@emcbot emcbot added CI-Gaeac6-Passed **Bot use only** CI testing on Gaea C6 for this PR has completed successfully CI-Hercules-Passed **Bot use only** CI testing on Hercules for this PR has completed successfully and removed CI-Gaeac6-Running **Bot use only** CI testing on Gaea C6 for this PR is in-progress CI-Hercules-Running **Bot use only** CI testing on Hercules for this PR is in-progress labels Dec 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI-Gaeac6-Passed **Bot use only** CI testing on Gaea C6 for this PR has completed successfully CI-Hercules-Passed **Bot use only** CI testing on Hercules for this PR has completed successfully CI-Ursa-Running **Bot use only** CI testing on Ursa for this PR is in-progress CI-Wcoss2-Running CI testing on WCOSS for this PR is in-progress

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants