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

Change order of type params to register_method #312

Merged
merged 3 commits into from
May 12, 2021

Conversation

dvdplm
Copy link
Contributor

@dvdplm dvdplm commented May 11, 2021

I think it reads a bit better with the return type being the first type param and the second anonymous, i.e. register_method::<MyReturnType, _>(… …) than register_method::<_, MyReturnType>(… …). 

@dvdplm dvdplm requested a review from insipx May 11, 2021 19:58
Copy link
Contributor

@insipx insipx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, yeah second as anonymous is better imo

EDIT: I approved this but it seems like CI doesn't like the style, what's the point of B::Two?

ws-server/src/tests.rs Outdated Show resolved Hide resolved
@niklasad1 niklasad1 merged commit d6b6bef into master May 12, 2021
@niklasad1 niklasad1 deleted the dp-type-params-order branch May 12, 2021 09:10
dvdplm added a commit that referenced this pull request May 18, 2021
* Add a test for calling methods with multiple params of multiple types (#308)

* Add a test for calling methods with multiple params of multiple types

* cargo fmt

Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>

* [ws client] RegisterNotification support (#303)

* Rename NotifResponse to SubscriptionResponse to make room for new impl

* Add support for on_notification Subscription<T> types

* Fix handling of NotificationHandler in manager

* cleanup

* Implement NotificationHandler to replace Subscription<T> and clean up plumbing

* More cleanup

* impl Drop for NotificationHandler

* Address pr feedback #1

* ws client register_notification pr feedback 2

* Fix doc

* fix typo

* Add tests, get NH working

* More cleanup of String/&str

* fix doc

* Drop notification handler on send_back_sink error

* ws client notification auto unsubscribe when channel full test

* Change order of type params to register_method (#312)

* Change order of type params to register_method

* Cleanup and fmt

* Update ws-server/src/tests.rs

Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>

* CI: optimize caching (#317)

* Bump actions/checkout from 2 to 2.3.4 (#315)

Bumps [actions/checkout](https://github.com/actions/checkout) from 2 to 2.3.4.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v2...v2.3.4)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump actions-rs/cargo from 1 to 1.0.3 (#314)

Bumps [actions-rs/cargo](https://github.com/actions-rs/cargo) from 1 to 1.0.3.
- [Release notes](https://github.com/actions-rs/cargo/releases)
- [Changelog](https://github.com/actions-rs/cargo/blob/master/CHANGELOG.md)
- [Commits](actions-rs/cargo@v1...v1.0.3)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump actions-rs/toolchain from 1 to 1.0.7 (#313)

Bumps [actions-rs/toolchain](https://github.com/actions-rs/toolchain) from 1 to 1.0.7.
- [Release notes](https://github.com/actions-rs/toolchain/releases)
- [Changelog](https://github.com/actions-rs/toolchain/blob/master/CHANGELOG.md)
- [Commits](actions-rs/toolchain@v1...v1.0.7)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* [ws server]: add logs (#319)

* WIP - hangs

* fix example

* cleanup

* Add certificate_store() to WsClientBuilder (#321)

* Add custom_certificate to WsClientBuilder

* Use system certs instead of specified file

* Cache client_config

* Move client_config logic to fn build

* Default use_system_certificates to true

* Move out connector

* Add CertificateStore type

* cargo fmt

* cargo clippy

* Resolve comment: Rename variable

* Resolved comments

Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>
Co-authored-by: Billy Lindeman <billylindeman@gmail.com>
Co-authored-by: Denis Pisarev <denis.pisarev@parity.io>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Albin Hedman <albin9604@gmail.com>
dvdplm added a commit that referenced this pull request May 28, 2021
* [ws server]: draft SubscriptionSinkWithParams

* rexport types

* PoC design2

* improve example

* Update ws-server/src/server.rs

Co-authored-by: David <dvdplm@gmail.com>

* Subscription example (#324)

* Add a test for calling methods with multiple params of multiple types (#308)

* Add a test for calling methods with multiple params of multiple types

* cargo fmt

Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>

* [ws client] RegisterNotification support (#303)

* Rename NotifResponse to SubscriptionResponse to make room for new impl

* Add support for on_notification Subscription<T> types

* Fix handling of NotificationHandler in manager

* cleanup

* Implement NotificationHandler to replace Subscription<T> and clean up plumbing

* More cleanup

* impl Drop for NotificationHandler

* Address pr feedback #1

* ws client register_notification pr feedback 2

* Fix doc

* fix typo

* Add tests, get NH working

* More cleanup of String/&str

* fix doc

* Drop notification handler on send_back_sink error

* ws client notification auto unsubscribe when channel full test

* Change order of type params to register_method (#312)

* Change order of type params to register_method

* Cleanup and fmt

* Update ws-server/src/tests.rs

Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>

* CI: optimize caching (#317)

* Bump actions/checkout from 2 to 2.3.4 (#315)

Bumps [actions/checkout](https://github.com/actions/checkout) from 2 to 2.3.4.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v2...v2.3.4)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump actions-rs/cargo from 1 to 1.0.3 (#314)

Bumps [actions-rs/cargo](https://github.com/actions-rs/cargo) from 1 to 1.0.3.
- [Release notes](https://github.com/actions-rs/cargo/releases)
- [Changelog](https://github.com/actions-rs/cargo/blob/master/CHANGELOG.md)
- [Commits](actions-rs/cargo@v1...v1.0.3)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump actions-rs/toolchain from 1 to 1.0.7 (#313)

Bumps [actions-rs/toolchain](https://github.com/actions-rs/toolchain) from 1 to 1.0.7.
- [Release notes](https://github.com/actions-rs/toolchain/releases)
- [Changelog](https://github.com/actions-rs/toolchain/blob/master/CHANGELOG.md)
- [Commits](actions-rs/toolchain@v1...v1.0.7)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* [ws server]: add logs (#319)

* WIP - hangs

* fix example

* cleanup

* Add certificate_store() to WsClientBuilder (#321)

* Add custom_certificate to WsClientBuilder

* Use system certs instead of specified file

* Cache client_config

* Move client_config logic to fn build

* Default use_system_certificates to true

* Move out connector

* Add CertificateStore type

* cargo fmt

* cargo clippy

* Resolve comment: Rename variable

* Resolved comments

Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>
Co-authored-by: Billy Lindeman <billylindeman@gmail.com>
Co-authored-by: Denis Pisarev <denis.pisarev@parity.io>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Albin Hedman <albin9604@gmail.com>

* grumbles: impl maciej proposal

* fix test build

* add test for subscription with param

* cargo fmt

* Update examples/ws_subscription.rs

Co-authored-by: David <dvdplm@gmail.com>

* Update utils/src/server/rpc_module.rs

Co-authored-by: David <dvdplm@gmail.com>

* Update utils/src/server/rpc_module.rs

Co-authored-by: David <dvdplm@gmail.com>

* Update utils/src/server/rpc_module.rs

Co-authored-by: David <dvdplm@gmail.com>

* Update utils/src/server/rpc_module.rs

Co-authored-by: David <dvdplm@gmail.com>

* Update utils/src/server/rpc_module.rs

Co-authored-by: David <dvdplm@gmail.com>

* grumbles

* Update utils/src/server/rpc_module.rs

Co-authored-by: David <dvdplm@gmail.com>

* Update utils/src/server/rpc_module.rs

Co-authored-by: David <dvdplm@gmail.com>

* Update utils/src/server/rpc_module.rs

Co-authored-by: Maciej Hirsz <1096222+maciejhirsz@users.noreply.github.com>

* fix more grumbles

* [subscriptionSink]: introduce into_sinks

* use replace

* fix more nits

* maciej design 2

* fix tests

* remove log

* [rpc context mod]: register_subscription with ctx

* nits

* nits again

* move subscribers mutex

* clippy

* [ws subscribe]: avoid send message on unsubscribed

* revert unintentional changes

* Subscription with context example (#345)

* Add weather example to show how to use subscriptions with context

* Add note

* Cleanup

* Additional cleanup (#347)

* Add weather example to show how to use subscriptions with context

* Add note

* Cleanup

* fmt

* Cleanup and docs

* fmt

* ignore error on subscription

Co-authored-by: David Palm <dvdplm@gmail.com>
Co-authored-by: Billy Lindeman <billylindeman@gmail.com>
Co-authored-by: Denis Pisarev <denis.pisarev@parity.io>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Albin Hedman <albin9604@gmail.com>
Co-authored-by: Maciej Hirsz <1096222+maciejhirsz@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants