fix(ts): provide complete and correct types#385
fix(ts): provide complete and correct types#385callmehiphop merged 7 commits intogoogleapis:masterfrom callmehiphop:dg--complete-types
Conversation
googlebot
added
the
cla: yes
|
Uh, is this a breaking change for TypeScript users? |
|
@JustinBeckwith maybe? I'm not really sure how to classify changing interfaces that were flat out wrong. |
|
Hah, I hear you. For now let's call it semver patch I guess? |
|
@JustinBeckwith sounds good.. like I said in the overview my only concern would be about some fields being marked optional now. But I could probably just use a generic to mark them as non-optional until we're ready for a semver major. |
Codecov Report
@@ Coverage Diff @@
## master #385 +/- ##
==========================================
+ Coverage 99.44% 99.45% +<.01%
==========================================
Files 4 4
Lines 544 547 +3
Branches 75 75
==========================================
+ Hits 541 544 +3
Misses 2 2
Partials 1 1
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #385 +/- ##
==========================================
+ Coverage 99.45% 99.45% +<.01%
==========================================
Files 4 4
Lines 547 550 +3
Branches 76 76
==========================================
+ Hits 544 547 +3
Misses 2 2
Partials 1 1
Continue to review full report at Codecov.
|
callmehiphop
added
do not merge
kokoro-team
removed
the
kokoro:force-run
bcoe
left a comment
bcoe
left a comment
There was a problem hiding this comment.
If I'm understanding @JustinBeckwith's comments correctly, and we're copying over TypeScript definitions from another-repo, I think it's worthwhile adding a build script that copies these over from the other library -- we'll be thankful in the future when the two libraries don't drift apart.
As for whether this is a breaking change; I'd argue, even if the old behavior is a bug, it's probably worth calling it a breaking change; especially since this library is already > 1.0.0; is it a huge deal if folks start getting [email protected]?
bcoe
left a comment
bcoe
left a comment
There was a problem hiding this comment.
👍 sounds like it would be a hassle to copy the types over; please do what you think is best @callmehiphop.
callmehiphop
added
the
do not merge
|
There were a couple of names that were a little off, since we're going to release this as a major bump I went ahead and changed them. |
callmehiphop
removed
🚨
JustinBeckwith
added
the
automerge
6 participants
Fixes #332
This basically rewrites all the types. Pretty much every request/response interface was missing fields and every callback/response returned
request.Responsewhich we don't return anywhere.In an effort to not make breaking changes, I've kept all type names and duplicate types in tact.