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: allow sam validate to look built template #2647

Closed
wants to merge 1 commit into from

Conversation

sriram-mv
Copy link
Contributor

Which issue(s) does this change fix?

#2613

Why is this change necessary?

  • sam validate only looks for current template.yaml which maybe out of date, if a build has been run.

How does it address the issue?

  • Directly check for a built template.yaml if present, sam build generates this template.

What side effects does this change have?

  • None

Checklist

  • Add input/output type hints to new functions/methods
  • Write design document (Do I need to write a design document?)
  • Write unit tests
  • Write/update functional tests
  • Write/update integration tests
  • make pr passes
  • make update-reproducible-reqs if dependencies were changed
  • Write documentation

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

Base automatically changed from develop to elbayaaa-develop March 25, 2021 21:28
Base automatically changed from elbayaaa-develop to develop March 25, 2021 21:38
@jfuss
Copy link
Contributor

jfuss commented Apr 14, 2021

@sriram-mv Is there any kind of tests we need to include for this?

@Rabadash8820
Copy link
Contributor

Just came across this PR from aws/serverless-application-model#1907. Any updates? Also out of curiosity, what will be the intended functionality now if we run sam validate before sam build? Will build be run automatically, or will validate just fail with a more informative error message?

@sriram-mv sriram-mv closed this Aug 25, 2021
@Rabadash8820
Copy link
Contributor

@sriram-mv Why was this PR closed without merging?

@sriram-mv
Copy link
Contributor Author

@Rabadash8820 : #2892 looks to be related to this and was the reason it was closed, Let me re-open and investigate to ensure.

@sriram-mv sriram-mv reopened this Aug 26, 2021
@jfuss
Copy link
Contributor

jfuss commented Mar 3, 2022

This is solved by #2892, from my understanding. If this that is not the case, please open a new Issue, as the one linked by this PR is closed.

Closing

@jfuss jfuss closed this Mar 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants