-
Notifications
You must be signed in to change notification settings - Fork 61
STITCH-2539: Fix the JS-Linter #235
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
Conversation
tkaye407
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To make this PR/CR more manageable I commented in the few spots where you should take a look at what is going on.
packages/browser/core/src/core/internal/net/BrowserFetchStreamTransport.ts
Show resolved
Hide resolved
packages/browser/core/src/core/internal/net/BrowserFetchStreamTransport.ts
Show resolved
Hide resolved
adamchel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This generally LGTM, thanks so much for taking care of this, and thanks a lot for adding the comments, they were super helpful!
I would do the E2E test that I mentioned to make sure redirect state is cleaned, and please answer my other questions but otherwise this will be good to go.
packages/browser/core/src/core/internal/net/BrowserFetchStreamTransport.ts
Show resolved
Hide resolved
packages/server/core/src/core/internal/net/NodeFetchStreamTransport.ts
Outdated
Show resolved
Hide resolved
packages/core/sdk/src/services/internal/CoreStitchServiceClientImpl.ts
Outdated
Show resolved
Hide resolved
| // sourced from https://github.com/coolaj86/TextEncoderLite | ||
| // Sourced from https://github.com/coolaj86/TextEncoderLite | ||
| /** @hidden */ | ||
| /* tslint:disable */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds good to me, though we should probably include the license of where we sourced this from
adamchel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
No description provided.