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

CI GitHub pipeline (hotfix) update for fetching repo name #3084

Conversation

TerrenceMcGuinness-NOAA
Copy link
Collaborator

@TerrenceMcGuinness-NOAA TerrenceMcGuinness-NOAA commented Nov 8, 2024

Description

Simple bug fix to GitHub CI pipeline script for Parallel Works.
Introduced a bug on its last update for setting the repo name in advance of running the GitHub CLI command for getting the repo owner and branch name of PRs.

Type of change

  • Bug fix (fixes something broken)
  • New feature (adds functionality)
  • 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?

How has this been tested?

Gets tested when update to default repo gets checked in.

Comment on lines 75 to 77
repo={{ github.repository }}
if [ "$pr_number" -eq "0" ]; then
branch=${{ github.event.inputs.ref }}
repo=${{ github.repository }}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a little confused by the syntax used here with {{}} and $. Is this Jinja or is this specific syntax for GitHub workflows?

To retain consistency, I'm guessing that this should be

Suggested change
repo={{ github.repository }}
if [ "$pr_number" -eq "0" ]; then
branch=${{ github.event.inputs.ref }}
repo=${{ github.repository }}
repo=${{ github.repository }}
if [ "$pr_number" -eq "0" ]; then
branch=${{ github.event.inputs.ref }}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes ${{ gitsutff }} is specific GitHub syntax, and yes line 75 was a typo

Copy link
Collaborator Author

@TerrenceMcGuinness-NOAA TerrenceMcGuinness-NOAA left a comment

Choose a reason for hiding this comment

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

committing type suggestion

Comment on lines 75 to 77
repo={{ github.repository }}
if [ "$pr_number" -eq "0" ]; then
branch=${{ github.event.inputs.ref }}
repo=${{ github.repository }}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes ${{ gitsutff }} is specific GitHub syntax, and yes line 75 was a typo

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!

@DavidHuber-NOAA DavidHuber-NOAA merged commit e6df3b3 into NOAA-EMC:develop Nov 12, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI/CD Issue related to CI/CD
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants