Skip to content

Conversation

@medikoo
Copy link
Contributor

@medikoo medikoo commented Apr 15, 2021

configurationPath was introduced in first refactors as mid step, still more natural in programmatic usage is to refer to serviceDir and eventually configurationFilename (but not many use cases for that one)

In the end eventual service configuration to be passed to Serverless constructor will consist of following options:

  • configuration - Service configuration object
  • serviceDir - Base path for resolution of any paths that are configured in service configuration
  • configurationFilename - Needed just in some cases (1) Error reporting (when we want to mention specifically configuration filename) (2) To update service configuration programmatically (e.g. to add plugin to plugins section with plugin install command). (As internals will become more and more modular, this property may appear at some point as optional)

(Support for configurationPath in scope of v2, will be maintained to not break things)

Settled on serviceDir, not servicePath (which was used so far in most places), to indicate that in all cases we deal with directory, also it'll match baseDir setting to be proposed as part of a solution for mulitple service repo handling and sourceDir which we plan to introduce with #7920

Additionally:

  • Replace all internal serverless.config.servicePath occurences with serverless.serviceDir (this attributes to Deprecate "serverless.config" #8836)
  • Rename all servicePath variables to serviceDir in codebase.
  • Refactor packaging test to async/await (observed one issue, and switching from execSync to async spawn helped to see internally what's going on)

@medikoo medikoo self-assigned this Apr 15, 2021
@medikoo medikoo changed the title API: Replace configurationPath with serviceDir and configurationFilename API: Introduce serviceDir and configurationFilename constructor options Apr 15, 2021
@medikoo medikoo force-pushed the 0414-introduce-service-dir branch 3 times, most recently from fa626b0 to 0000d8e Compare April 16, 2021 09:49
@codecov
Copy link

codecov bot commented Apr 16, 2021

Codecov Report

Merging #9307 (27416de) into master (d667111) will increase coverage by 0.04%.
The diff coverage is 91.26%.

❗ Current head 27416de differs from pull request most recent head 0bdb1b8. Consider uploading reports for the commit 0bdb1b8 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9307      +/-   ##
==========================================
+ Coverage   86.87%   86.91%   +0.04%     
==========================================
  Files         316      316              
  Lines       11767    11773       +6     
==========================================
+ Hits        10223    10233      +10     
+ Misses       1544     1540       -4     
Impacted Files Coverage Δ
lib/classes/Variables.js 99.73% <ø> (ø)
lib/cli/resolve-configuration-path.js 96.42% <ø> (ø)
lib/plugins/aws/customResources/index.js 98.57% <ø> (ø)
lib/plugins/aws/deploy/lib/extendedValidate.js 100.00% <ø> (ø)
lib/plugins/aws/deploy/lib/uploadArtifacts.js 90.00% <ø> (ø)
lib/plugins/aws/deployFunction.js 96.62% <ø> (ø)
...ib/plugins/aws/package/lib/generateCoreTemplate.js 95.34% <ø> (ø)
...ib/plugins/aws/package/lib/saveCompiledTemplate.js 100.00% <ø> (ø)
lib/plugins/aws/package/lib/saveServiceState.js 100.00% <ø> (ø)
lib/plugins/plugin/uninstall.js 100.00% <ø> (ø)
... and 32 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d667111...0bdb1b8. Read the comment docs.

@medikoo medikoo force-pushed the 0414-introduce-service-dir branch from 0000d8e to 0bdb1b8 Compare April 16, 2021 10:29
@medikoo medikoo requested a review from pgrzesik April 16, 2021 10:41
Copy link
Contributor

@pgrzesik pgrzesik left a comment

Choose a reason for hiding this comment

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

Looks great, well done 👍

@medikoo medikoo merged commit 712a569 into master Apr 16, 2021
@medikoo medikoo deleted the 0414-introduce-service-dir branch April 16, 2021 11:32
medikoo added a commit that referenced this pull request Apr 20, 2021
medikoo added a commit that referenced this pull request Apr 20, 2021
wwwehr pushed a commit to wwwehr/serverless that referenced this pull request Apr 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants