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

Provide Consistent Service Path (Fix #5242) #5314

Merged
merged 3 commits into from Oct 2, 2018

Conversation

@erikerikson
Copy link
Member

commented Sep 20, 2018

What did you implement:

Fix #5242

How did you implement it:

Do this by first supplying the servicePath in ~/lib/plugins/lib/validate.js and ~/lib/plugins/print.js in a consistent manner with the code in ~/lib/classes/PluginManager.js. Then, provide a default of process.cwd() for servicePath in getCacheFilePath and getServerlessConfigFile.

How can we verify it:

As per #5242, this bug impacts the execution of Serverless with a servicePath that is not the current working directory. One could re-create this circumstance.

I have offered a new test to validate this behavior in getServerlessConfigFile.test.js, the corresponding change in getCacheFilePath.js seemed too trivial to warrant the code. Can provide that too if desired.

Paging @eahefnawy as per discussion in issue #5242

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

erikerikson added 2 commits Sep 20, 2018
Fix #5242

Do this by first supplying the `servicePath` in `~/lib/plugins/lib/validate.js` and `~/lib/plugins/print.js` in a consistent manner with the code in `~/lib/classes/PluginManager.js`.  Then, provide a default of `process.cwd()` for `servicePath` in `getCacheFilePath` and `getServerlessConfigFile`.
@shanehandley shanehandley referenced this pull request Sep 21, 2018
6 of 6 tasks complete
@erikerikson

This comment has been minimized.

Copy link
Member Author

commented Sep 29, 2018

If merged, please squash.

@horike37 horike37 added this to the 1.33.0 milestone Oct 2, 2018
Copy link
Member

left a comment

@erikerikson
It works. LGTM 👍

@horike37 horike37 merged commit 83f1460 into serverless:master Oct 2, 2018
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.002%) to 91.051%
Details
@erikerikson erikerikson deleted the erikerikson:consistent-service-path branch Oct 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.