Skip to content

Conversation

@erikerikson
Copy link
Contributor

What did you implement:

Fixes reported issue on #4754 by @dschep

Handle self:service.[...] references properly. The prior represents what the user typed into their configuration. In service, we move all this from service to serviceObject, therefore, by rewriting self:service.[...] to self:serviceObject.[...] user's will get the result they expect.
Cleanup: use else if, given exclusivity of conditions

How did you implement it:

filter self:service. variables, replacing that part with self:serviceObject. to avoid failed resolutions.

How can we verify it:

Run the offered test.

Todos:

  • Write tests
  • [n/a] 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

Handle `self:service.[...]` references properly.  The prior represents what the user typed into their configuration.  In service, we move all this from `service` to `serviceObject`, therefore, by rewriting `self:service.[...]` to `self:serviceObject.[...]` user's will get the result they expect.
Use else if, given exclusivity of conditions
Copy link
Contributor

@HyperBrain HyperBrain left a comment

Choose a reason for hiding this comment

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

Looks good from my side regarding the change.
As soon as we have a confirmation that it actually fully resolves the problem, we can merge it

@dschep
Copy link
Contributor

dschep commented Mar 28, 2018

Works like a charm! Thanks @erikerikson!

@horike37 horike37 added this to the 1.27 milestone Mar 29, 2018
@horike37 horike37 merged commit 721f8b6 into serverless:master Mar 29, 2018
@erikerikson erikerikson deleted the self-service-rewrite branch August 20, 2019 05:16
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.

4 participants