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

Interactive CLI interactive hook #6549

Merged
merged 5 commits into from
Aug 26, 2019
Merged

Conversation

dschep
Copy link
Contributor

@dschep dschep commented Aug 15, 2019

What did you implement:

Added interactiveCli:end lifecycle hook to enable serverless/enterprise-plugin#226

How did you implement it:

added 'end' lifecycle hook to the interactiveCli plugin

How can we verify it:

checkout this branch and use the plugin from the other pr and try the sls interactive flow, you should get the messages at the end.

Todos:

Note: Run npm run test-ci to run all validation checks on proposed changes

  • Write tests and confirm existing functionality is not broken.
    Validate via npm test
  • Write documentation
  • Ensure there are no lint errors.
    Validate via npm run lint-updated
    Note: Some reported issues can be automatically fixed by running npm run lint:fix
  • Ensure introduced changes match Prettier formatting.
    Validate via npm run prettier-check-updated
    Note: All reported issues can be automatically fixed by running npm run prettify-updated
  • 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

@dschep dschep requested a review from medikoo August 15, 2019 16:17
@dschep dschep force-pushed the interactivecli-interactive-hook branch from 80a9647 to 049434b Compare August 15, 2019 20:45
Copy link
Contributor

@medikoo medikoo left a comment

Choose a reason for hiding this comment

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

It looks that unrelated commit sneaked into this

@dschep dschep force-pushed the interactivecli-interactive-hook branch from 049434b to a0ed7eb Compare August 16, 2019 11:24
@dschep dschep dismissed medikoo’s stale review August 16, 2019 11:24

removed docs from othe rpr

@dschep dschep requested a review from medikoo August 16, 2019 11:24
medikoo
medikoo previously approved these changes Aug 16, 2019
Copy link
Contributor

@medikoo medikoo left a comment

Choose a reason for hiding this comment

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

It appears we need to first publish @serverless/enterprise-plugin.

Also I wouldn't bump any versions here.. as this requires cherry-picked release branch preparation (we already have some new features in master after 1.50.0 was released)

Copy link
Contributor

@pmuens pmuens left a comment

Choose a reason for hiding this comment

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

Looks good from a code perspective. Is there anything we should test here? Not sure if a test would be superfluous...

Otherwise I'd say it's GTM :shipit:

@pmuens pmuens added this to the 1.51.0 milestone Aug 26, 2019
@pmuens pmuens changed the title Interactivecli interactive hook Interactive CLI interactive hook Aug 26, 2019
pmuens
pmuens previously requested changes Aug 26, 2019
Copy link
Contributor

@pmuens pmuens left a comment

Choose a reason for hiding this comment

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

Also I wouldn't bump any versions here.. as this requires cherry-picked release branch preparation (we already have some new features in master after 1.50.0 was released)

Update the review as it makes sense to keep the 1.50.0 version as @medikoo mentioned above.

package.json Outdated
@@ -1,6 +1,6 @@
{
"name": "serverless",
"version": "1.50.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

We could keep the old version here to avoid cherry-picking as @medikoo mentioned in his PR review.

@dschep dschep dismissed pmuens’s stale review August 26, 2019 12:37

version numbers fixed

@dschep dschep merged commit 843650d into master Aug 26, 2019
dschep added a commit that referenced this pull request Aug 26, 2019
* PLAT-1391 - add interactiveCli:end lifecycle hook

(cherry picked from commit 843650d)
dschep added a commit that referenced this pull request Aug 26, 2019
* Interactive CLI interactive hook (#6549)

* PLAT-1391 - add interactiveCli:end lifecycle hook

(cherry picked from commit 843650d)

* bump version numbers

* 1.51.1 changelog
@medikoo medikoo removed this from the 1.51.0 milestone Aug 28, 2019
@medikoo medikoo deleted the interactivecli-interactive-hook branch August 29, 2019 08:18
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