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

Add plans and payment methods to Pro client #1340

Merged
merged 7 commits into from
Dec 22, 2023
Merged

Conversation

atavism
Copy link
Contributor

@atavism atavism commented Dec 13, 2023

No description provided.

@coveralls
Copy link

Coverage Status

coverage: 55.679% (-1.5%) from 57.192%
when pulling a58dbd8 on atavism/desktop-updates
into 7314dfd on main.

@myleshorton
Copy link
Contributor

I'm a little confused about this one @atavism. Is this because this is currently done in lantern-client? Does the pro server actually support these protocol buffers requests and responses?

@myleshorton
Copy link
Contributor

Or are you taking advantage of the fact that the proto definition can also support JSON responses?

@atavism
Copy link
Contributor Author

atavism commented Dec 20, 2023

Is this because this is currently done in lantern-client? Does the pro server actually support these protocol buffers requests and responses?

@myleshorton, we actually have a few different Pro client implementations, and I thought we could update Android and desktop to both use this one. And that's right: the protocol buffer requests and responses are the same that are being used on the pro server.

Or are you taking advantage of the fact that the proto definition can also support JSON responses?

Yeah, just being able to generate the structs and JSON from the protocol buffer definitions I feel is really useful as well. If you think this is overkill though, I am fine with removing it!

@myleshorton
Copy link
Contributor

Ok LGTM!

@atavism atavism merged commit 2306b47 into main Dec 22, 2023
1 check failed
@atavism atavism deleted the atavism/desktop-updates branch December 22, 2023 11:30
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.

3 participants