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

fix(common): Cannot find module HttpUploadProgressEvent #30852

Conversation

@santoshyadav198613
Copy link
Contributor

commented Jun 4, 2019

Fixes #30814

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Issue Number: #30814

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@santoshyadav198613 santoshyadav198613 requested a review from angular/fw-http as a code owner Jun 4, 2019

@googlebot googlebot added the cla: yes label Jun 4, 2019

@ngbot ngbot bot added this to the needsTriage milestone Jun 4, 2019

@santoshyadav198613 santoshyadav198613 force-pushed the santoshyadav198613:fix(common)--Cannot-find-module-response branch 2 times, most recently from daf221b to 03591d1 Jun 4, 2019

@santoshyadav198613

This comment has been minimized.

Copy link
Contributor Author

commented Jun 5, 2019

Hi @gkalpak ,
Need some help here, not able to figure oy why tests are getting failed, it just says size is too big.

@gkalpak

This comment has been minimized.

Copy link
Member

commented Jun 5, 2019

Thx for working on this, @santoshyadav198613. I don't see a size-related failure. The one I see (that matters) is about updating the public API guard golden file. You can look at the output in CircleCI#346811 for more details.

Basically, since you have modified the public API for @angular/common/http, you need to also update the golden file: tools/public_api_guard/common/http.d.ts. Either by running bazel run //tools/public_api_guard:common_http_api.accept (if that works on your machine 😁) or by manually making the following change:

+export interface HttpUploadProgressEvent extends HttpProgressEvent {
+    type: HttpEventType.UploadProgress;
+}
+
 export declare class HttpUrlEncodingCodec implements HttpParameterCodec {
@gkalpak

gkalpak approved these changes Jun 5, 2019

Copy link
Member

left a comment

Thx!

One minor comment:
Since the commit message will show up in the changelog, I think it would be more clear for the reader to mention what the commit did. E.g.:

fix(common): expose the `HttpUploadProgressEvent` interface as public API

@santoshyadav198613 santoshyadav198613 force-pushed the santoshyadav198613:fix(common)--Cannot-find-module-response branch from 03591d1 to c5334a8 Jun 5, 2019

@santoshyadav198613 santoshyadav198613 requested a review from angular/fw-public-api as a code owner Jun 5, 2019

@santoshyadav198613

This comment has been minimized.

Copy link
Contributor Author

commented Jun 5, 2019

Thx for working on this, @santoshyadav198613. I don't see a size-related failure. The one I see (that matters) is about updating the public API guard golden file. You can look at the output in CircleCI#346811 for more details.

Basically, since you have modified the public API for @angular/common/http, you need to also update the golden file: tools/public_api_guard/common/http.d.ts. Either by running bazel run //tools/public_api_guard:common_http_api.accept (if that works on your machine ) or by manually making the following change:

+export interface HttpUploadProgressEvent extends HttpProgressEvent {
+    type: HttpEventType.UploadProgress;
+}
+
 export declare class HttpUrlEncodingCodec implements HttpParameterCodec {

Thanks @gkalpak ,

You are a savior I was struggling with this, worked now.

@santoshyadav198613

This comment has been minimized.

Copy link
Contributor Author

commented Jun 5, 2019

Thx!

One minor comment:
Since the commit message will show up in the changelog, I think it would be more clear for the reader to mention what the commit did. E.g.:

fix(common): expose the HttpUploadProgressEvent interface as public API

Done.

@ngbot

This comment has been minimized.

Copy link

commented Jun 5, 2019

I see that you just added the PR action: merge label, but the following checks are still failing:
    failure status "ci-codefresh-bazel" is failing
    pending status "google3" is pending

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken master, please try rebasing to master and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

@gkalpak

This comment has been minimized.

Copy link
Member

commented Jun 5, 2019

Thx, @santoshyadav198613 👍 (Don't worry about the codefresh CI failure. It is unrelated to this PR.)
Now, we just need approval from @angular/fw-http and @angular/fw-public-api (cc @IgorMinar, @alxhub)

@IgorMinar

This comment has been minimized.

Copy link
Member

commented Jun 6, 2019

merge-assistance: code-fresh flake.

@mhevery mhevery closed this in 5c18f23 Jun 7, 2019

mhevery added a commit that referenced this pull request Jun 7, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.