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

Add tencent provider create-template #6898

Merged
merged 7 commits into from Oct 29, 2019
Merged

Add tencent provider create-template #6898

merged 7 commits into from Oct 29, 2019

Conversation

anycodes
Copy link
Contributor

What did you implement

Add Tencent provider create-template

How can we verify it

  • select a template, for example: tencent-python3
  • run cd tencent-python3 and create credentials vim ~/credentials, input your secretId, secretKey, appid:
[default]
tencent_appid = appid
tencent_secret_id = secretid
tencent_secret_key = secretkey
  • deploy: sls deploy

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.

@anycodes Thank you, looks good! Before we merge just wanted to clarify on few things (see comments)

@@ -0,0 +1,6 @@
# package directories
Copy link
Contributor

Choose a reason for hiding this comment

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

Node.js v6 is already at EOL. Do you feel there's a value in providing template for it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you. I have updated the template.

"author": "cloud.tencent.com",
"license": "MIT",
"devDependencies": {
"serverless-tencent-scf": "*"
Copy link
Contributor

Choose a reason for hiding this comment

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

It'll be good to pin it to current version so ^0.1.7. That will prevent any eventual breaking changes to a plugin to affect the template

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you.
I have added the plugin version in package.json just now.

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.

Thanks a lot for working on this @anycodes 👍

Overall this looks good. I only have some points we might want to discuss before merging this into master:

  1. Should we add tests to ensure that all the necessary files are created? Such tests can be found in create.test.js (/cc @medikoo)
  2. Should we only include one runtime such as tencent-python which creates a template for the most recent version and add a comment in the template itself that the user can also use e.g. Python3.6? This way we only need to introduce one template per runtime and don't need to append the version in the template name
  3. Do we want to omit Node 6 and Python 2 since they're deprecated / will be deprecated soon? (as @medikoo mentioned above)

medikoo
medikoo previously approved these changes Oct 29, 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.

Looks great! Thank you @anycodes

@medikoo
Copy link
Contributor

medikoo commented Oct 29, 2019

@anycodes just noticed, there are some style issues. Can you run npm run prettify-updated and commit changes?

@anycodes
Copy link
Contributor Author

DFOUNDERLIU-MB0:serverless dfounderliu$ npm run prettify-updated

serverless@1.55.1 prettify-updated /Users/dfounderliu/Desktop/temp/serverless
pipe-git-updated --ext=css --ext=html --ext=js --ext=json --ext=md --ext=yaml --ext=yml -- prettier --write

lib/plugins/create/create.js 135ms
lib/plugins/create/create.test.js 362ms
lib/plugins/create/templates/tencent-go/package.json 20ms
lib/plugins/create/templates/tencent-go/serverless.yml 58ms
lib/plugins/create/templates/tencent-nodejs/index.js 10ms
lib/plugins/create/templates/tencent-nodejs/package.json 6ms
lib/plugins/create/templates/tencent-nodejs/serverless.yml 16ms
lib/plugins/create/templates/tencent-php/package.json 8ms
lib/plugins/create/templates/tencent-php/serverless.yml 13ms
lib/plugins/create/templates/tencent-python/package.json 15ms
lib/plugins/create/templates/tencent-python/serverless.yml 12ms

@medikoo medikoo added this to the 1.56.0 milestone Oct 29, 2019
@medikoo medikoo removed the feature label Oct 29, 2019
@medikoo medikoo merged commit 593ef90 into serverless:master Oct 29, 2019
@medikoo medikoo deleted the add-tencent branch October 29, 2019 15:41
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.

None yet

3 participants