-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
TypeScript support for the existing Node.js example #5325
TypeScript support for the existing Node.js example #5325
Conversation
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
Codecov Report
@@ Coverage Diff @@
## master #5325 +/- ##
=======================================
Coverage 71.80% 71.80%
=======================================
Files 392 392
Lines 14228 14228
=======================================
Hits 10217 10217
Misses 3259 3259
Partials 752 752 Continue to review full report at Codecov.
|
0f24f10
to
0106c13
Compare
@googlebot I fixed it. |
I see Travis tests mostly pass, except for
|
Thanks for the contribution @onpaws , I believe there's an issue with the
|
@IsaacPD the |
The test have that are failing have been temporarily disabled on master, could you please rebase to catch these changes and prevent travis from failing. Thanks |
Yep, you got it, coming right up |
e67cf4c
to
28692a7
Compare
Hopefully this is the desired commit: Let's see if the tests pass this time 🤞 Update: hooray, looks like they did 🥳 |
Hi there, I'm Pat. First time contributor. Thanks for your efforts on skaffold!
Fixes: #4261
Description
The example Node.js project is a great starting point for JavaScript developers. Some developers may prefer to use TypeScript, so this PR intends to do the minimum number of steps to port the existing Node.js example to TypeScript, making sure hot reload still works in the expected way.
Steps I took:
examples/nodejs
and renamed toexamples/typescript
npm install typescript
tsc --init
and accepted all defaults.@types/express
and@types/node
) to keep the compiler happy.Follow-up Work
[VS Code] Potentially consider remote debugging
Tests
I didn't write new tests because I'm not sure what exactly it should mean to "test" an example.
That being said
make test
seems to still pass:Same for
make integration
-- the newexamples/typescript
folder did show up in the output and it appears to pass also. (I'm not going to pretend I understand what this test suite is actually doing :) Maybe it copies all the examples intoexamples/
?)Versions