fix(common): add params and report progress to put - #37873
ajitsinghkaler wants to merge 1 commit into
Conversation
jelbourn
left a comment
There was a problem hiding this comment.
LGTM
Reviewed-for: public-api
|
Do we need a google3 presubmit on this?? |
petebacondarwin
left a comment
There was a problem hiding this comment.
This seems like a perfectly reasonable fix unless someone can explain why this particular overload did not have these options? It looks like a simple oversight to me.
There are some typos in the commit message. Can you change it to:
fix(common): add `params` and `reportProgress` options to `HttpClient.put()` overload
When the response type is JSON, the `put()` overload signature did not have `reportProgress`
and `params` options. This makes it difficult to type-check this overload.
This commit adds them to the overload signature.
Fixes #23600
bedc480 to
5c8aeff
Compare
|
@petebacondarwin changed the message |
petebacondarwin
left a comment
There was a problem hiding this comment.
Reviewed-for: public-api-approvers
atscott
left a comment
There was a problem hiding this comment.
Reviewed-for: public-api
AndrewKushnir
left a comment
There was a problem hiding this comment.
Reviewed-for: public-api
|
FYI, the remaining group that needs to give an LGTM is |
|
@AndrewKushnir is there a problem with the presubmit. Should I change something in this. |
|
Hi @ajitsinghkaler, I've started a new presubmit and will let you know how it goes. Meanwhile, could you please rebase this PR so we re-run CI for this change with the most up-to-date version of master branch? Thank you. |
….put()` overload When the response type is JSON, the `put()` overload signature did not have `reportProgress` and `params` options. This makes it difficult to type-check this overload. This commit adds them to the overload signature. Fixes angular#23600
5c8aeff to
f80c3d3
Compare
|
@AndrewKushnir rebased it. |
|
Hi @ajitsinghkaler, the presubmit went well. It looks like this PR has all approvals, so I'm adding the "merge" label. Thank you. |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
When response type is Json the put sigbnature did not have report progress and params key. This makes it difiicult to type check added them to the signature
Fixes #23600
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #23600
What is the new behavior?
Does this PR introduce a breaking change?
Other information