-
Notifications
You must be signed in to change notification settings - Fork 186
Fixes #182 Allow overriding appspec path + fallback to appspec.yml and appspec.yaml #265
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice change and good job with the tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎸 🤘
Removed leftover debug typos Renamed variable to not have default in the name Added arg --appspec-path Allow for appspec-filename to be overridden
@yubangxi @rohkat-aws or @brblck do you guys mind re-running the travis build on the PR as I added new tests? Seems reopening/closing or new commits don't help. |
@vrr-21 I was wondering if you'd be able to review this one? I merged upstream changes from master so the conflicts are now resolved. |
This is a great change, testing on our end then we'll get this merged in. |
What
Fixes #182: Allow for .yaml extension for appspec.
What is actually happening under the hood
Now we allow for a new parameter:
--appspec-filename
Which allows us to specify (and thus override) an alternative name to the previously hardcoded
appspec.yml
file.If the
--appspec-filename
variable is not set, then it defaults to:appspec.yml
(which is the old value)Additionally, if this parameter is set but we fail to find the file it points to in the root dir, we fallback to (in that order):
appspec.yaml
appspec.yml
Acknowledgement of License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Additional notes:
@yubangxi if we are merging this, it'd be a good idea to also update the info in the doc to reflect that.