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

Change cookiecutter URL to point at correct github repo #692

Closed
wants to merge 1 commit into from

Conversation

stealthycoin
Copy link

Removed the hello in the github reference since that repo does not exist.

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

@jfuss
Copy link
Contributor

jfuss commented Oct 9, 2018

@stealthycoin These are not the same repos. The one you have changed to is an advanced python example and not the hello world one that is in the repo. The cookiecutter parts of this README do not make sense to me, since they will be created through SAM CLI. @heitorlessa Why is the README saying to use cookiecutter instead of SAM CLI?

@jfuss jfuss added the stage/in-progress A fix is being worked on label Oct 9, 2018
@stealthycoin
Copy link
Author

In any case the one I was changing it from is a 404: https://github.com/aws-samples/cookiecutter-aws-sam-hello-python. Someone else had reported that the SAM CLI when used to run the template didn't set up the testing requirements properly (pytest etc) so I was trying out the cookiecutter template directly to see if it generated them. If it was actually a different template that I wound up running, it would make sense why it had the test dependencies declared and the other one didn't.

@jfuss
Copy link
Contributor

jfuss commented Oct 9, 2018

That github doesn't exist and we should fix this doc but we can't change that reference the one you have since it is completely different.

My point is that I think we should just remove those lines you changed but wanted to understand why it is the way it is now before moving forward.

@heitorlessa
Copy link
Contributor

@jfuss This is an unpaid tech debt. README comes from unpublished repo before sam init was created; we're creating dedicated repo for each runtime as initially intended and moving these outside this repo.

What customers should be reading today is not this README but the README that is generated as part of the template as these are raw files.

@jfuss
Copy link
Contributor

jfuss commented Oct 10, 2018

@heitorlessa That doesn't mean we should keep it either though. If it does not provide any value, we should just remove it. Thoughts?

@heitorlessa
Copy link
Contributor

heitorlessa commented Oct 10, 2018 via email

@jfuss
Copy link
Contributor

jfuss commented Jan 8, 2019

@heitorlessa Are you talking these types of things in with your rework of all the init examples? Do we still need this if that is the case?

@heitorlessa
Copy link
Contributor

heitorlessa commented Jan 9, 2019 via email

@jfuss
Copy link
Contributor

jfuss commented Jan 18, 2019

@stealthycoin Thanks for the PR. We addressed this in a different PR (#849) while redoing some of the init examples. Closing this as it is no longer needed.

@jfuss jfuss closed this Jan 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stage/in-progress A fix is being worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants