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 #2997 #3186

Merged
merged 1 commit into from Feb 23, 2017
Merged

fix #2997 #3186

merged 1 commit into from Feb 23, 2017

Conversation

dannycohn
Copy link
Contributor

@dannycohn dannycohn commented Feb 3, 2017

What did you implement:

Closes #2997

How did you implement it:

I moved the block of code the ensures that all functions have names out of the load function and into its own function that can be called by Serverless.js after variables have been expanded.

How can we verify it:

Use ${file(functions.json)} to load your functions array from a file

Todos:

  • Write tests
  • Write documentation
  • Fix linting errors
  • Make sure code coverage hasn't dropped
  • Provide verification config / commands / resources
  • Enable "Allow edits from maintainers" for this PR
  • Update the messages below

Is this ready for review?: YES
Is it a breaking change?: NO

@dannycohn
Copy link
Contributor Author

I have updated the tests and they are all passing. I'm not sure if this is the best way to fix this issue, but it's what made sense to me. Please review

@dannycohn
Copy link
Contributor Author

@eahefnawy @pmuens

I don't want to be annoying, just curious, do I need to do anything else to have this PR reviewed, or is it just a matter of someone getting to it?

@pmuens
Copy link
Contributor

pmuens commented Feb 7, 2017

Hey @dannycohn thank you very much for taking the time to work on a fix for #2997 ! 👍

There's no need to do anything else right now! The only thing important is that the PR template is filled out (what you've already done).

Right now we don't have an ETA for this issue, but we'll look into it once we plan the next milestones.

BTW. Everyone here is encouraged to review PRs 💯 .
This reduces the "Time-To-Merge" and might also increase the quality of PRs since it's always better to have different opinions from people who are using Serverless!

@eahefnawy
Copy link
Member

Looks good @dannycohn 👍

@eahefnawy eahefnawy merged commit 74ceec5 into serverless:master Feb 23, 2017
@eahefnawy eahefnawy added this to the 1.8 milestone Feb 23, 2017
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

3 participants