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

fix: do not add duplicate inputs #446

Merged
merged 6 commits into from
Aug 15, 2022
Merged

Conversation

Nr18
Copy link
Contributor

@Nr18 Nr18 commented Mar 8, 2022

Issue #, if available: #445

Description of changes:

When using the param_overrides functionality. And you use 2 parameters from the same source. The artifact is added twice to the action. This causes the pipeline to fail.

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

Joris Conijn added 2 commits March 8, 2022 11:56
When using the `param_overrides` functionality. And you use 2 parameters from the same source. The artifact is added twice to the action. This causes the pipeline to fail.

Issue: awslabs#445
@Nr18 Nr18 force-pushed the fix/duplicate-inputs branch from 3bde993 to b88c7a2 Compare March 8, 2022 11:11
@Nr18 Nr18 force-pushed the fix/duplicate-inputs branch from b88c7a2 to 5d6bdb2 Compare March 8, 2022 12:18
Copy link
Collaborator

@sbkok sbkok 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 providing a fix for this issue.

Could you please add test cases to the following file [0] to verify that this works correctly?
That way we know it will continue to work in the future, even if this part gets refactored.

@Nr18 Nr18 requested a review from sbkok March 11, 2022 10:39
@Nr18
Copy link
Contributor Author

Nr18 commented Mar 11, 2022

@sbkok I added 2 tests:

  • 1 to make sure that inputs are not added twice (the original fix)
  • 2 when using different inputs it should be added.

Also added some context on how to run the top build locally.

Copy link
Collaborator

@sbkok sbkok left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for adding the tests too.

@javydekoning javydekoning self-requested a review August 15, 2022 09:14
Copy link
Contributor

@javydekoning javydekoning left a comment

Choose a reason for hiding this comment

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

Manually ran our latest CI on this. All checks are passing.

PR LGTM! Thanks for contributing.

@sbkok sbkok merged commit 9c383d0 into awslabs:master Aug 15, 2022
@sbkok sbkok added this to the v3.2.0 milestone Jan 24, 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.

3 participants