Skip to content

Add deprecation message for slss alias#8156

Merged
medikoo merged 3 commits into
serverless:masterfrom
crash7:build-deprecate-slss
Sep 1, 2020
Merged

Add deprecation message for slss alias#8156
medikoo merged 3 commits into
serverless:masterfrom
crash7:build-deprecate-slss

Conversation

@crash7

@crash7 crash7 commented Aug 29, 2020

Copy link
Copy Markdown
Contributor

Addresses: #8122

@crash7 crash7 mentioned this pull request Aug 29, 2020
Closed
2 tasks
@codecov-commenter

codecov-commenter commented Aug 29, 2020 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #8156 into master will decrease coverage by 0.09%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8156      +/-   ##
==========================================
- Coverage   88.28%   88.18%   -0.10%     
==========================================
  Files         248      248              
  Lines        9429     9444      +15     
==========================================
+ Hits         8324     8328       +4     
- Misses       1105     1116      +11
Impacted Files Coverage Δ
lib/utils/processBackendNotificationRequest.js 38.46% <0.00%> (-32.97%) ⬇️
lib/Serverless.js 97.33% <0.00%> (-1.32%) ⬇️
lib/plugins/standalone/index.js 31.37% <0.00%> (-1.24%) ⬇️
lib/utils/logDeprecation.js 100.00% <0.00%> (ø)
lib/utils/analytics/generatePayload.js 100.00% <0.00%> (ø)

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 c45be69...89847ee. Read the comment docs.

Bennyhwanggggg
Bennyhwanggggg previously approved these changes Aug 31, 2020
View reviewed changes
medikoo
medikoo suggested changes Aug 31, 2020
View reviewed changes

@medikoo medikoo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you! That looks promising. Please see my remarks.

We need to also document this deprecation here: https://github.com/serverless/serverless/blob/master/docs/deprecations.md

Comment thread scripts/serverless.js Outdated
Comment thread scripts/serverless.js Outdated
@medikoo

medikoo commented Aug 31, 2020

Copy link
Copy Markdown
Contributor

I've just realized that this detection won't work on windows (process.argv[1] will always point resolved script filename).

Having that I believe we need to introduce a temporary slss.js file in bin folder, put deprecation there, make it require serverless.js and point slss to it.

@crash7

crash7 commented Aug 31, 2020

Copy link
Copy Markdown
Contributor Author

I've just realized that this detection won't work on windows (process.argv[1] will always point resolved script filename).

Having that I believe we need to introduce a temporary slss.js file in bin folder, put deprecation there, make it require serverless.js and point slss to it.

Good catch, totally forgot about windows. Going to add the dummy script then.

@crash7
crash7 requested a review from medikoo August 31, 2020 21:59
medikoo
medikoo suggested changes Sep 1, 2020
View reviewed changes

@medikoo medikoo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@crash7 thanks that looks good, I have just two improvement proposals. Please see my comments

Comment thread bin/slss.js Outdated
Comment thread docs/deprecations.md Outdated
@medikoo medikoo added cat/cli deprecation Deprecation proposal (breaking with next major) labels Sep 1, 2020
@crash7
crash7 requested a review from medikoo September 1, 2020 13:03
medikoo
medikoo approved these changes Sep 1, 2020
View reviewed changes

@medikoo medikoo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks great! Thank you @crash7

@medikoo
medikoo merged commit a2d1031 into serverless:master Sep 1, 2020
medikoo pushed a commit that referenced this pull request Sep 1, 2020
@crash7 @medikoo
feat(CLI): Deprecate "slss" alias (#8156)
0b4de09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers
@medikoo medikoo medikoo approved these changes
@Bennyhwanggggg Bennyhwanggggg Bennyhwanggggg left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@crash7 crash7

Labels

cat/cli deprecation Deprecation proposal (breaking with next major)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@crash7 @codecov-commenter @medikoo @Bennyhwanggggg