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 typescript express example #577

Merged
merged 9 commits into from
Feb 2, 2021
Merged

Add typescript express example #577

merged 9 commits into from
Feb 2, 2021

Conversation

hoseungme
Copy link
Contributor

@hoseungme hoseungme commented Jan 24, 2021

Hi. I added a pretty simple express application example with typescript.
Thanks!

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.

Thanks @hoseungjang, it looks good, however, as I'm not expert on Typescript, I'll see if we can get another review. Also, please see a few of my comments

aws-node-typescript-express/README.md Outdated Show resolved Hide resolved
aws-node-typescript-express/serverless.yml Outdated Show resolved Hide resolved
aws-node-typescript-express/src/app.ts Outdated Show resolved Hide resolved
@pgrzesik
Copy link
Contributor

Thank you @hoseungjang - I believe it might be a bit better if you'd move your example to your own repository and list it under Community Examples - would you be okay with that?

@hoseungme
Copy link
Contributor Author

Hi @pgrzesik
I don't mind that opinion, but could you tell me about the reason of it?

@pgrzesik
Copy link
Contributor

Sure @hoseungjang - as you're the only author of this example, it feels to me that it belongs more into Community Examples. Additionally, I'm not a TypeScript expert, which makes it a bit harder for me to assess the quality and confidently introduce it as a part of this repository.

@hoseungme
Copy link
Contributor Author

hoseungme commented Jan 31, 2021

Thank you for answer @pgrzesik .
Then how can I move this example to Community Examples? just add it to community-examples.json?
and can I do this job on this PR? or should I open a new one?

@pgrzesik
Copy link
Contributor

pgrzesik commented Feb 1, 2021

Yes, you should add it to the mentioned file. You can totally do that as a part of this PR. Thank you 🙇

@hoseungme
Copy link
Contributor Author

Hi @pgrzesik !
I'm all done. Could you check it again?
But I'm sorry that it may take a long time to complete CI, it may be because I live far from you.

@pgrzesik
Copy link
Contributor

pgrzesik commented Feb 2, 2021

Thank you @hoseungjang - I've forgot that the README has to be updated manually, sorry about that. Could you run npm run docs locally so it will update the README.md file with your entry?

Thanks 🙇

@hoseungme
Copy link
Contributor Author

Thank you @pgrzesik .
Could you check again please?

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.

Thank you @hoseungjang 🙇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants