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

Rework setting image tags to support PRs better #161

Merged
merged 1 commit into from
Nov 6, 2024
Merged

Conversation

webbnh
Copy link
Contributor

@webbnh webbnh commented Nov 4, 2024

Changes introduced with this PR

This PR reworks the reusable workflow which builds plugin container images. It combines into one all the steps which handle different cases of generating container image tags depending on the CI trigger. And, if the trigger is a Pull Request, it uses the name of the PR branch instead of the name of the GitHub "ref" (which is usually something generic, like 17/merge). And, it picks a few bits of lint.

So, the tags will now be one of the following:

  • releases: release and latest [this is unchanged]
  • the main branch: main_latest [this is unchanged]
  • PRs: <PR_branch_name>_<last_commit_hash> [this is new]
  • tags and other branches: <tag-name_or_branch-ref>_<commit_hash> [this is unchanged]

This PR was tested using arcalot/arcaflow-plugin-rtla#5 for PR builds and for tag builds, and it will use arcalot/arcaflow-plugin-rtla#4 for testing main builds (i.e., once this PR is merged, then that one can be merged, and then this one will get tested 🙂).


By contributing to this repository, I agree to the contribution guidelines.

Copy link

@dbutenhof dbutenhof left a comment

Choose a reason for hiding this comment

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

Yeah, OK, this seems plausible, although I kinda wish it was easier to follow this in the workflow log. (But the values on Quay look fine, so the IMAGE_TAGS setting clearly propagated to the action and did what we expected.)

@webbnh
Copy link
Contributor Author

webbnh commented Nov 4, 2024

I kinda wish it was easier to follow this in the workflow log.

Yeah; I did make it show its work (ditto) for you, though.

@webbnh webbnh merged commit c7c36bb into main Nov 6, 2024
3 checks passed
@webbnh webbnh deleted the name-PR-image-tags branch November 6, 2024 20:27
@webbnh
Copy link
Contributor Author

webbnh commented Nov 6, 2024

it will use arcalot/arcaflow-plugin-rtla#4 for testing main builds (i.e., once this PR is merged, then that one can be merged, and then this one will get tested 🙂).

And, it did, and it worked!

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.

3 participants