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

Update hello world ruby template #860

Closed
wants to merge 5 commits into from
Closed

Update hello world ruby template #860

wants to merge 5 commits into from

Conversation

lauratpa
Copy link

First of all, thank you for this wonderful tool!

I tried out the hello world app for ruby, and it works nicely! But I discovered that the test that was written for this example isn't capable of running.

Description of changes:

  • Update the test, so that it tests the functionality and passes
  • Put testing gems to the Gemfile, so that we can use bundler to manage these gems
  • Remove whitespace

Checklist:

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

@@ -44,7 +44,7 @@ rvm --default use 2.5.0
```bash
gem install bundler
bundle install
bundle install --deployment --path hello_world/vendor/bundle
bundle install --deployment --path hello_world/vendor/bundle --without test
Copy link

Choose a reason for hiding this comment

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

@lauratpa should bundle install also skip development?

bundle install --deployment --path hello_world/vendor/bundle --without development test

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, it makes sense that one does not want development gems in production deployment. But I didn't add it because the example Gemfile does not have development group and I don't want to confuse anyone. Makes sense?

@awood45
Copy link
Member

awood45 commented Dec 18, 2018

I'm looking at incorporating this into another PR I'm working on right now - or it will need to be rebased. The current project structure from sam init doesn't work with sam build, so some files are gonna get moved...

awood45 added a commit to awood45/aws-sam-cli that referenced this pull request Dec 18, 2018
Global Gemfile for tests. Gemfile in the function for use in tests.

Hat tip to lauratpa and her PR aws#860 which is the core of this part of
the change set.
@awood45
Copy link
Member

awood45 commented Dec 18, 2018

#866 has pulled in the changes in this PR, generally.

@sriram-mv
Copy link
Contributor

Thanks for making the changes! these look to be good. I see #866 has pulled in these changes along with adding support for ruby in sam build.

@lauratpa
Copy link
Author

@awood45 @thesriram I see that my changes were pulled to the new PR. I'll close this PR now in that case. Thanks, waiting to try out the feature!

@lauratpa lauratpa closed this Dec 18, 2018
@lauratpa lauratpa deleted the update-ruby-template branch December 18, 2018 06:25
@awood45
Copy link
Member

awood45 commented Dec 18, 2018

Thank you so much for improving the examples!

sriram-mv pushed a commit to awood45/aws-sam-cli that referenced this pull request Dec 20, 2018
Global Gemfile for tests. Gemfile in the function for use in tests.

Hat tip to lauratpa and her PR aws#860 which is the core of this part of
the change set.
sriram-mv pushed a commit that referenced this pull request Dec 21, 2018
* Add Ruby to Supported Runtime Enum

* Ruby Project Template Changes

WIP: Unit tests may have a notion of a central Gemfile that isn't
compatible with per-function Gemfile pattern that `sam build` expects.

* Tweaks

Global Gemfile for tests. Gemfile in the function for use in tests.

Hat tip to lauratpa and her PR #860 which is the core of this part of
the change set.

* Ruby Init App in Working State

* fix: add integration tests for sam build - ruby

* bump: version bump aws-lambda-builders to 0.0.5

* fix: requests version pin and imports

* fix: string paths on Pathlib for py2

* fix: README - add references to build
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.

None yet

4 participants