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

Refactor inputs to support docker/metadata-action #76

Merged
merged 2 commits into from Oct 12, 2021
Merged

Refactor inputs to support docker/metadata-action #76

merged 2 commits into from Oct 12, 2021

Conversation

ntkme
Copy link
Contributor

@ntkme ntkme commented Oct 6, 2021

This PR adds support for docker/metadata-action@v2. See #74.

Description

Support tags input in full image name form. When full image name form is used, input image would be optional.

Related Issue(s)

Closes #74

Checklist

  • This PR includes a documentation change
  • This PR does not need a documentation change

  • This PR includes test changes
  • This PR's changes are already tested

  • This change is not user-facing
  • This change is a patch change
  • This change is a minor change
  • This change is a major (breaking) change

Changes made

  • Input image is now declared as required: false, and checked during runtime.
  • If all tags are in full image name form, the tags are used as is.
  • tags input are split by /\s+/ instead of " " because docker/metadata-action's output is newline separated.

Copy link
Member

@divyansh42 divyansh42 left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution 👍
I think this PR is very accurate and perfect as per implementation.
The only thing that needs to be done is to update the input description (as we have changed those) in action.yml and in README.

@divyansh42
Copy link
Member

divyansh42 commented Oct 7, 2021

Do you want to update/improve the documentation to clearly explain the changes? (In the same way, suggested at redhat-actions/push-to-registry#50 (comment))

@ntkme
Copy link
Contributor Author

ntkme commented Oct 8, 2021

@tetchel Done with refactoring and added documentation. PTAL. Thanks!

Copy link
Member

@divyansh42 divyansh42 left a comment

Choose a reason for hiding this comment

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

Output image will be empty when using FQIN, can we add a note about this in the README output section?
Also, it will be great if we can add another example for FQIN in output tags and image-with-tag in README output section.

@tetchel tetchel changed the title Support docker/metadata-action Refactor inputs to support docker/metadata-action Oct 12, 2021
@tetchel
Copy link
Contributor

tetchel commented Oct 12, 2021

lgtm, i'm going to hold this until the PTR one is approved too and then we can merge both and make sure they make sense together.

@tetchel tetchel merged commit 979e6a6 into redhat-actions:main Oct 12, 2021
@tetchel tetchel mentioned this pull request Oct 12, 2021
8 tasks
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.

[FEATURE] Compatibility with docker/metadata-action
3 participants