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

[sdk] Improve API & internal state #7414

Merged
merged 3 commits into from
Sep 2, 2024
Merged

[sdk] Improve API & internal state #7414

merged 3 commits into from
Sep 2, 2024

Conversation

charlag
Copy link
Contributor

@charlag charlag commented Aug 16, 2024

We changed the point at which the credentials are fixed in the sdk. Now they are passed in to login() method and not when creating SDK. This makes for a cleaner separation of a stateless, shared SDK and stateful, account-bound LoggedInSDK. Rest resources are now always bound to a session which allows for parallel logins and prevents accidental bugs

let entity_client = Arc::new(EntityClient::new(
self.rest_client.clone(),
self.json_serializer.clone(),
self.base_url.clone(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps pass this by reference?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would be great! but we would need to define the lifetime relationship between Sdk and EntityClient and I think Uniffi types are not allowed to have lifetimes.

Copy link
Contributor

@rezbyte rezbyte left a comment

Choose a reason for hiding this comment

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

The changes are an improvement for sure! I am curious whether we could pass Strings by reference more often.

@charlag charlag force-pushed the dev-mail branch 2 times, most recently from c763e8c to def698a Compare August 30, 2024 16:16
paw-hub and others added 3 commits September 2, 2024 10:26
We changed the point at which the credentials are fixed in the sdk. Now
they are passed in to login() method and not when creating SDK. This
makes for a cleaner separation of a stateless, shared SDK and stateful,
account-bound LoggedInSDK. Rest resources are now always bound to a
session which allows for parallel logins and prevents accidental bugs
@charlag charlag merged commit 1d170b9 into dev-mail Sep 2, 2024
4 checks passed
@charlag charlag deleted the sdk-state branch September 2, 2024 09:08
@charlag charlag added state:done meets our definition of done no-release-notes (excluded from release notes) state:not-testable "tested" for blind fixes or old code removal with no expected regressions labels Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-release-notes (excluded from release notes) state:done meets our definition of done state:not-testable "tested" for blind fixes or old code removal with no expected regressions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants