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

Support tags #28

Merged
merged 4 commits into from
Feb 8, 2023
Merged

Support tags #28

merged 4 commits into from
Feb 8, 2023

Conversation

yyamanoi1222
Copy link
Contributor

*Issue #21

Description of changes:
Add tags input

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@DmitryGulin
Copy link
Contributor

Thank you very much for your PR!

I'd recommend simplifying the tag defintion format.

The current approach expects action authors to use an array of Tag compatible objects, so that the defintion would look like this:

[{ "Key": "first_tag", "The first tag value": "Tag Value" }, { "Key": "second_tag", "Value": "The second tag value" }]

The current approach is too verbose and unnecessary exposes internal dependency on AWS SDK API as a public contract.
That said, I'd suggest using a simplified version to make action defintion easier for action authors to create, understand, and support:

{ "first_tag": "The first tag value", "second_tag": "The second tag value" }

Copy link
Contributor

@DmitryGulin DmitryGulin left a comment

Choose a reason for hiding this comment

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

see my comment in the Conversation thread

@yyamanoi1222
Copy link
Contributor Author

@DmitryGulin Thank you for your feedbacks!
I changed the format

@yyamanoi1222 yyamanoi1222 requested review from DmitryGulin and removed request for hariohmprasath January 19, 2023 10:55
@DmitryGulin DmitryGulin merged commit c20d75f into awslabs:main Feb 8, 2023
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.

2 participants