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

move InternalClientOptions and NodeInitializationParameters to types module #725

Merged
merged 2 commits into from Jan 13, 2020

Conversation

@pedrouid
Copy link
Member

pedrouid commented Jan 13, 2020

Trying to assign IChannelProvider to InternalClientOptions and NodeInitializationParameters required to export it from a different file so as a quick solution I added to the channelProvider.ts as follows:|

export type IIChannelProvider = IChannelProvider

However this is super hacky so I moved both types to the types module and ended up refactoring a bunch of stuff but it's definitely cleaner and easier to use them for the test-runner module

@todo-tracker

This comment has been minimized.

Copy link

todo-tracker bot commented Jan 13, 2020

Hey, pedrouid

We noticed you made changes to a file with a TODO on it.
These are set to make sure potential Technical Debt doesn't get forgotten.
While you're here take a shot at turning a listed TODO into a TODONE!

Id Name File Priority
71 merge: modules/client/src/node.ts Normal
78 why is it sometimes data vs data.data? modules/client/src/controllers/AbstractController.ts Normal

button

@LayneHaber LayneHaber self-requested a review Jan 13, 2020
Copy link
Contributor

LayneHaber left a comment

This makes it easier to use it in the test runner cc @bohendo

also, i like having all the types in types

@pedrouid pedrouid merged commit 9fc98f8 into 703-channelProvider-tests Jan 13, 2020
6 of 7 checks passed
6 of 7 checks passed
build
Details
test-cf
Details
test-cf
Details
test-client
Details
test-integration test-integration
Details
test-client
Details
test-node
Details
@pedrouid pedrouid deleted the move-types branch Jan 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.