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: Change build-image CLI argument to files #825

Merged
merged 4 commits into from
Dec 8, 2021

Conversation

liamcoatman
Copy link
Contributor

Issue #, if available:

Description of changes:

Change argument name from file to files, which is what is read on lines 56 and 58. I've tested this manually and verified that only machine image foo is built when I run pnpx sls build-image --files foo.json -s dev.

Checklist:

  • Have you successfully deployed to an AWS account with your changes?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully tested with your changes locally?
  • If new dependencies have been added, have they been pinned to specific versions?
  • Is this change also required on the AWS Solution version?
  • Have you updated openapi.yaml if you made updates to API definition (including add, delete or update parameter and request data schema)?
  • If you had to run manual tests, have you considered automating those tests by adding them to end-to-end tests?
  • If you are updating the changelog and vending out a new release, have you updated versionNumber and versionDate in .defaults.yml

AS review ticket id:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@liamcoatman liamcoatman requested a review from a team as a code owner November 24, 2021 15:51
@SanketD92 SanketD92 changed the base branch from mainline to develop December 2, 2021 15:43
Copy link
Contributor

@SanketD92 SanketD92 left a comment

Choose a reason for hiding this comment

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

Could you provide more details as to what didn't work earlier with the "file" argument?

@codecov
Copy link

codecov bot commented Dec 2, 2021

Codecov Report

Merging #825 (ef2e351) into develop (772617f) will not change coverage.
The diff coverage is n/a.

❗ Current head ef2e351 differs from pull request most recent head 43bb068. Consider uploading reports for the commit 43bb068 to get more accurate results
Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #825   +/-   ##
========================================
  Coverage    50.91%   50.91%           
========================================
  Files          286      286           
  Lines        15931    15931           
  Branches      2485     2485           
========================================
  Hits          8111     8111           
  Misses        6863     6863           
  Partials       957      957           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 772617f...43bb068. Read the comment docs.

@SanketD92 SanketD92 added the closing-soon-if-no-response Ask for response from requester, will close issue in 7 days if no response label Dec 3, 2021
@liamcoatman
Copy link
Contributor Author

Could you provide more details as to what didn't work earlier with the "file" argument?

As far as I could tell it it does nothing. An argument like --file foo.json is ignored and the default (to build all Packer files in the directory) is used instead.

@SanketD92 SanketD92 removed the closing-soon-if-no-response Ask for response from requester, will close issue in 7 days if no response label Dec 6, 2021
@SanketD92
Copy link
Contributor

Could you provide more details as to what didn't work earlier with the "file" argument?

As far as I could tell it it does nothing. An argument like --file foo.json is ignored and the default (to build all Packer files in the directory) is used instead.

Was able to verify the --files flag was honored while the --flag flag wasn't.

Please pull the latest changes from develop by clicking on the "Update branch" button below. Also, please add a commit (could even be an empty one) that starts with a conventional commit type.

@SanketD92 SanketD92 self-assigned this Dec 7, 2021
@liamcoatman liamcoatman changed the title Fix: Change build-image CLI argument to files fix: Change build-image CLI argument to files Dec 8, 2021
@SanketD92 SanketD92 merged commit 7506895 into awslabs:develop Dec 8, 2021
jxuamazon pushed a commit to jxuamazon/service-workbench-on-aws that referenced this pull request Feb 15, 2022
* fix: Change build-image CLI argument to files
Co-authored-by: Liam Coatman <liam.c@faculty.ai>
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