refactor(AWS Deploy): Deprecate `function` option in `deploy` command #9364

jkklapp · 2021-04-24T17:00:07Z

codecov · 2021-04-24T17:03:51Z

Codecov Report

Merging #9364 (58d7d51) into master (34a9d91) will decrease coverage by 0.00%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master    #9364      +/-   ##
==========================================
- Coverage   86.14%   86.14%   -0.01%     
==========================================
  Files         326      326              
  Lines       12436    12439       +3     
==========================================
+ Hits        10713    10715       +2     
- Misses       1723     1724       +1

Impacted Files	Coverage Δ
lib/cli/commands-schema/aws-service.js	`100.00% <ø> (ø)`
lib/plugins/deploy.js	`94.73% <66.66%> (-5.27%)`	⬇️

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 34a9d91...58d7d51. Read the comment docs.

pgrzesik

Thank you @jkklapp 🙇 It looks like a good start, but we should be more careful with removing functionalities that might break user workflows. In this case, we should just configure deprecation message, while keeping the old functionality and warn that it will be removed with next major version.

pgrzesik · 2021-04-27T08:58:35Z

test/unit/lib/plugins/deploy.test.js

      deploy.options.package = false;
      deploy.options.function = 'myfunc';
      deploy.serverless.service.package.path = false;

      return expect(deploy.hooks['before:deploy:deploy']()).to.be.fulfilled.then(() =>
-        BbPromise.all([


Let's not change this test - we still want to keep testing the old behavior until we remove it.

pgrzesik · 2021-04-27T08:59:29Z

test/unit/lib/plugins/deploy.test.js

@@ -79,18 +79,14 @@ describe('Deploy', () => {
      );
    });

-    it('should execute deploy function if a function option is given', () => {
+    it('should log a deprecation message if a function option is given', () => {


We usually don't test deprecation messages and I think we can skip test in this case as well. Otherwise, please configure the test with runServerless

pgrzesik · 2021-04-27T09:00:28Z

lib/plugins/deploy.js

+            'CLI_DEPLOY_FUNCTION_OPTION',
+            'The `--function` or `-f` option for `deploy` command has been r' +
+              'emoved. Please use `sls deploy function` command instead.'
+          );


This PR should only implement a deprecation notice, without removing the functionality.

pgrzesik · 2021-04-27T09:02:12Z

docs/deprecations.md

+
+<a name="CLI_DEPLOY_FUNCTION_OPTION"><div>&nbsp;</div></a>
+
+## CLI deploy command function option removed


Let's phrase the whole message so it explains that the flag has been deprecated and will be removed with next major (3.0.0).

pgrzesik

Thank you @jkklapp 🙇 I've added some suggestions to the deprecation message wording - please let me know what do you think 🙇

pgrzesik · 2021-05-03T08:03:08Z

docs/deprecations.md

@@ -363,3 +363,12 @@ Please use `onUnauthenticatedRequest` instead. `allowUnauthenticated` will be re
 Deprecation code: `BIN_SERVERLESS`

 Please use `bin/serverless.js` instead. `bin/serverless` will be removed with v2.0.0
+
+<a name="CLI_DEPLOY_FUNCTION_OPTION"><div>&nbsp;</div></a>


Let's put it at the top of the file, right below instructions on how to disable specific deprecations

pgrzesik · 2021-05-03T08:05:11Z

docs/deprecations.md

+
+Deprecation code: `CLI_DEPLOY_FUNCTION_OPTION'`
+
+The `--function` or `-f` option in `deploy` command has been deprecated and will be removed in the next major release (3.0.0). Please use


To follow the convention of other messages in deprecations, let's maybe go with

Starting with v3.0.0, `--function` or `-f` option for `deploy` command will be removed. In order to deploy a single function, please use `deploy function` command instead.

pgrzesik · 2021-05-03T08:05:59Z

lib/plugins/deploy.js

@@ -39,6 +39,11 @@ class Deploy {
          throw new ServerlessError(errorMessage);
        }
        if (this.options.function) {
+          this.serverless.logDeprecation(
+            'CLI_DEPLOY_FUNCTION_OPTION',
+            'The `--function` or `-f` option for `deploy` command has been' +


Let's use the same message as suggested in the deprecation document.

pgrzesik

Thank you @jkklapp - I've reviewed the implementation proposal in the original ticket again and I have two questions as the implementation differs a bit from the suggested approach - I would appreciate clarification on these 🙇

pgrzesik · 2021-05-03T09:11:20Z

lib/plugins/deploy.js

@@ -36,6 +36,10 @@ class Deploy {
          throw new ServerlessError(errorMessage, 'INVALID_PROVIDER');
        }
        if (this.options.function) {


I've consulted the implementation proposal in the ticket and I believe the suggestion was to implement it as a part of newly introduced initialize hook. Was there a reason to not follow the proposal here? The general approach is to emit deprecations as a part of initialize hooks

pgrzesik · 2021-05-03T09:11:51Z

lib/cli/commands-schema/aws-service.js

@@ -26,7 +26,7 @@ commands.set('deploy', {
      type: 'boolean',
    },
    'function': {
-      usage: "Function name. Deploys a single function (see 'deploy function')",
+      usage: "Function name. Deploys a single function. DEPRECATED: use 'deploy function' instead.",


According to proposal, it was suggested to remove this option from the schema - what was the reason against doing so?

It needs to stay in schema, otherwise it will not be properly recognized and the expected deprecation won't be reported

Yes, it's probably right to keep it (same way as keep schema for deprecated config properties)

medikoo

👍 Looks great

medikoo · 2021-06-29T11:49:07Z

lib/cli/commands-schema/aws-service.js

@@ -26,7 +26,7 @@ commands.set('deploy', {
      type: 'boolean',
    },
    'function': {
-      usage: "Function name. Deploys a single function (see 'deploy function')",
+      usage: "Function name. Deploys a single function. DEPRECATED: use 'deploy function' instead.",


Yes, it's probably right to keep it (same way as keep schema for deprecated config properties)

pgrzesik suggested changes Apr 27, 2021

View reviewed changes

jkklapp force-pushed the deprecation/function_option_in_deploy_command branch 3 times, most recently from 3b43652 to 998274a Compare April 30, 2021 17:09

jkklapp requested a review from pgrzesik April 30, 2021 17:15

pgrzesik suggested changes May 3, 2021

View reviewed changes

jkklapp requested a review from pgrzesik May 3, 2021 08:58

pgrzesik suggested changes May 3, 2021

View reviewed changes

jkklapp and others added 3 commits June 29, 2021 12:38

configure deprecation message for deploy function option

0d324ad

change wording in deprecation message

2367558

feat(AWS Deploy): Ensure that deprecation is properly triggered

58d7d51

pgrzesik force-pushed the deprecation/function_option_in_deploy_command branch from abba10f to 58d7d51 Compare June 29, 2021 11:18

pgrzesik added cat/deployment deprecation Deprecation proposal (breaking with next major) labels Jun 29, 2021

pgrzesik self-assigned this Jun 29, 2021

pgrzesik requested a review from medikoo June 29, 2021 11:27

medikoo approved these changes Jun 29, 2021

View reviewed changes

pgrzesik approved these changes Jun 29, 2021

View reviewed changes

pgrzesik changed the title ~~deprecate function option in deploy command~~ refactor(AWS Deploy): Deprecate function option in deploy command Jun 29, 2021

pgrzesik merged commit d861d11 into serverless:master Jun 29, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(AWS Deploy): Deprecate `function` option in `deploy` command #9364

refactor(AWS Deploy): Deprecate `function` option in `deploy` command #9364

jkklapp commented Apr 24, 2021

codecov bot commented Apr 24, 2021 •

edited

pgrzesik left a comment

pgrzesik Apr 27, 2021

pgrzesik Apr 27, 2021

pgrzesik Apr 27, 2021

pgrzesik Apr 27, 2021

pgrzesik left a comment

pgrzesik May 3, 2021

pgrzesik May 3, 2021

pgrzesik May 3, 2021

pgrzesik left a comment

pgrzesik May 3, 2021

pgrzesik May 3, 2021

pgrzesik Jun 29, 2021

medikoo Jun 29, 2021

medikoo left a comment

medikoo Jun 29, 2021


		<a name="CLI_DEPLOY_FUNCTION_OPTION"><div> </div></a>

		## CLI deploy command function option removed


		Deprecation code: `CLI_DEPLOY_FUNCTION_OPTION'`

		The `--function` or `-f` option in `deploy` command has been deprecated and will be removed in the next major release (3.0.0). Please use

refactor(AWS Deploy): Deprecate function option in deploy command #9364

refactor(AWS Deploy): Deprecate function option in deploy command #9364

Conversation

jkklapp commented Apr 24, 2021

codecov bot commented Apr 24, 2021 • edited

Codecov Report

pgrzesik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pgrzesik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pgrzesik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

medikoo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

refactor(AWS Deploy): Deprecate `function` option in `deploy` command #9364

refactor(AWS Deploy): Deprecate `function` option in `deploy` command #9364

codecov bot commented Apr 24, 2021 •

edited