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

feat: add workflow templates #1

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

gatici
Copy link

@gatici gatici commented Dec 20, 2024

Adding each job templates that can be used in any repository by providing input variables.
E2E tests are not included in this PR and it only includes the existing workflows.

@gatici gatici marked this pull request as draft December 20, 2024 11:43
@gatici
Copy link
Author

gatici commented Dec 20, 2024

This is still in-progress.

Copy link

@ghislainbourgeois ghislainbourgeois 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, I have some comments

@gatici gatici marked this pull request as ready for review March 13, 2025 21:46
Copy link

@gab-arrobo gab-arrobo left a comment

Choose a reason for hiding this comment

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

Overall, it looks good to me.
Just in case do you have an example of how you are planning to use these GHAs in the different repositories? I am asking this because I was expecting to see something like this

      branch-name:
-        description: Name of the branch to checkout
-        required: false
-        type: string
-        default: ${{ github.ref }}
+        - ${{ input_branch }}

Also, my understanding was that the workflow call is done from the perspective of the "calling" repository (e.g., amf, smf, config5g, etc.). That is, do we need to pass the branch name that we need to take action on?

@gatici gatici requested review from eb-oss and gab-arrobo March 18, 2025 09:54
@gatici
Copy link
Author

gatici commented Mar 18, 2025

Overall, it looks good to me. Just in case do you have an example of how you are planning to use these GHAs in the different repositories? I am asking this because I was expecting to see something like this

      branch-name:
-        description: Name of the branch to checkout
-        required: false
-        type: string
-        default: ${{ github.ref }}
+        - ${{ input_branch }}

Also, my understanding was that the workflow call is done from the perspective of the "calling" repository (e.g., amf, smf, config5g, etc.). That is, do we need to pass the branch name that we need to take action on?

Hello Gabriel,

I used the shared workflows in this PR omec-project/amf#374 and they run successfully. Please have a look.

Copy link

@gab-arrobo gab-arrobo left a comment

Choose a reason for hiding this comment

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

Overall, it looks very good. I added some comments for your review. Thanks!

else
echo "Failed to create release ${{ inputs.VERSION }} ❌" >> "$GITHUB_STEP_SUMMARY"
echo "Failed to create release ${{ inputs.VERSION }} ❌"
fi

Choose a reason for hiding this comment

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

Add end of line

Suggested change
fi
fi

if: steps.version-change.outputs.changed == 'true'
uses: ./actions/create-github-release
with:
VERSION: ${{ steps.version-change.outputs.version }}

Choose a reason for hiding this comment

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

Add end of line

Suggested change
VERSION: ${{ steps.version-change.outputs.version }}
VERSION: ${{ steps.version-change.outputs.version }}

token: ${{ secrets.GH_OMEC_PAT }}
commit-message: Update version
committer: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
author: ${{ github.actor }} <${{ github.actor_id }}+${{ github.actor }}@users.noreply.github.com>

Choose a reason for hiding this comment

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

I think we can remove this line because it just indicates the user who approved the "previous" PR

Suggested change
author: ${{ github.actor }} <${{ github.actor_id }}+${{ github.actor }}@users.noreply.github.com>

Comment on lines +25 to +27
git config --global user.email "github-actions[bot]@users.noreply.github.com"
git config --global user.name "GitHub Actions"
git add VERSION && git commit -m "Update VERSION file to $NEW_VERSION" && git push

Choose a reason for hiding this comment

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

Why do you need these lines? The next task is taking care of that (add, commit and push)

Suggested change
git config --global user.email "github-actions[bot]@users.noreply.github.com"
git config --global user.name "GitHub Actions"
git add VERSION && git commit -m "Update VERSION file to $NEW_VERSION" && git push


on:
workflow_run:
workflows: ["Tag GitHub"]

Choose a reason for hiding this comment

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

Does this have to use brackets or can it be a list such as below?

Suggested change
workflows: ["Tag GitHub"]
workflows:
- "Tag GitHub"

uses: actions/checkout@v4
with:
ref: ${{ inputs.branch_name }}
submodules: true

Choose a reason for hiding this comment

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

Why do you need to pull the submodules? I do not think any repo has a submodule

Suggested change
submodules: true

Comment on lines +26 to +27
- name: Get Dependencies
run: go mod download

Choose a reason for hiding this comment

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

Why do you need this step? We did not need it before


on:
workflow_run:
workflows: ["Tag GitHub"]

Choose a reason for hiding this comment

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

What about something like this

Suggested change
workflows: ["Tag GitHub"]
workflows:
- "Tag GitHub"

Comment on lines +20 to +22
- uses: peterjgrainger/[email protected]
with:
branch: "rel-${{ github.event.workflow_run.outputs.version_branch }}"
Copy link

@gab-arrobo gab-arrobo Mar 19, 2025

Choose a reason for hiding this comment

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

Just in case, have you checked what we can do regarding the creation of release branches (branch name is different than the content of the version file)? For example, in the amf repo, branch rel-1.3 has VERSION file with content 1.4.0

- name: "API request to create release"
shell: bash
run: |
if gh release create "${{ inputs.VERSION }}" --generate-notes; then

Choose a reason for hiding this comment

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

Should I understand that the GitHub runners have, by default, the GitHub client (gh) installed? Additionally, I should understand that this cannot be in the same GHA (i.e., tag-github) and that is why it is in its own directory (actions), correct?

@gab-arrobo gab-arrobo requested a review from thakurajayL March 19, 2025 04:05
@gab-arrobo
Copy link

@gatici,
BTW, I think we also need to add a Dependabot to handle dependency updates for the GitHub Action, correct?

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.

4 participants