Skip to content

Conversation

@erikerikson
Copy link
Contributor

What did you implement:

Only disable and restore dependent service resolution methods once. Otherwise, subsequent removals may cache the previous replacements. If they restore last then they will restore with the replacements, breaking standard usage.

Add tests for this guarantee.

This resolves the issue reported at #4754 (comment)

How did you implement it:

Rather than call off-limits method replacement for each dependent configuration, just do it once, process all the configuration that needs processing, and then do the restore.

How can we verify it:

npm run test
and with any valid serverless.yml:
sls print [<options>]

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

…therwise, subsequent removals may cache the previous replacements. If they restore last then they will restore with the replacements, breaking standard usage.

Add tests for this guarantee.
@erikerikson erikerikson mentioned this pull request Feb 24, 2018
6 tasks
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.

👍

@HyperBrain HyperBrain merged commit 74042d0 into serverless:master Feb 25, 2018
@pmuens pmuens added this to the 1.27 milestone May 2, 2018
@erikerikson erikerikson deleted the 4713-follow-up-fixes-2 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.

3 participants