Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

use service provider proto string impls #704

Merged
merged 1 commit into from
Jan 12, 2024

Conversation

andymck
Copy link
Contributor

@andymck andymck commented Jan 9, 2024

This makes use of centralised from_str and to_string impls added to the proto lib here helium/proto#386 and removes the temp added local functions.

There is the option of updating mobile_config server to return the Service Provider enum value but I am not convinced that is a good approach as it enforces a specific enum to what could be argued is a generic API (key_to_rewardable_entity) which translates a given key to a rewardable entity. Baking in the assumption that a rewardable entity will always be a service provider enum would be a mistake.

The other option is to drop the generic API (naming & usage ) and make it specific to returning a service provider entity key. I ended up taking this approach but limited it to the mobile config client. The server still returns the entity key as defined in the DB ( ie a string ), the client API then attempts to convert that to the relevant enum before passing the enum value to the caller. This approach allows the client to have APIs which return either the enum value or the raw string. I could well be overthinking this

Cargo.toml Outdated
@@ -60,14 +60,14 @@ sqlx = {version = "0", features = [
"runtime-tokio-rustls"
]}
helium-crypto = {version = "0.8.1", features=["sqlx-postgres", "multisig"]}
helium-proto = {git = "https://github.com/helium/proto", branch = "master", features = ["services"]}
helium-proto = {git = "https://github.com/helium/proto", branch = "andymck/sp-str-impls", features = ["services"]}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
helium-proto = {git = "https://github.com/helium/proto", branch = "andymck/sp-str-impls", features = ["services"]}
helium-proto = {git = "https://github.com/helium/proto", branch = "master", features = ["services"]}

@andymck andymck force-pushed the andymck/use-service-provide-enum-impls branch from 696323e to b0abc4c Compare January 12, 2024 10:43
@andymck andymck force-pushed the andymck/use-service-provide-enum-impls branch from b0abc4c to 9da46ab Compare January 12, 2024 11:11
@andymck andymck merged commit 835e59b into main Jan 12, 2024
1 check passed
@andymck andymck deleted the andymck/use-service-provide-enum-impls branch January 12, 2024 11:49
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants