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

feat: remove legacy sdk and related artifacts #265

Merged
merged 2 commits into from
May 7, 2024
Merged

Conversation

malandis
Copy link
Contributor

@malandis malandis commented May 6, 2024

With the imminent release of the new SDK, we remove the legacy SDK
from the code base, all related uitlities, tests, and examples.

With the imminent release of the new SDK, we remove the legacy SDK
from the code base, all related uitlities, tests, and examples.
@malandis malandis requested review from anitarua, cprice404 and a team May 6, 2024 23:00
@malandis malandis mentioned this pull request May 6, 2024
4 tasks
This is used by the CLI to log in via browser. In the future we can
add this directly in the CLI library as this is not related to the
other client libraries.
Copy link
Contributor

@anitarua anitarua left a comment

Choose a reason for hiding this comment

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

🧽

}
}

/// Initiate a login workflow.
Copy link
Contributor Author

@malandis malandis May 6, 2024

Choose a reason for hiding this comment

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

I feel this code belongs in the CLI as opposed to here. The CLI is the sole consumer of this. If we believe that it has broader utility and want to support it with our 1.0 guarantees I can add it back here.

cc @cprice404

/// # let get_response = Get::Hit { value: momento::response::GetValue::new(vec![]) };
/// # let get_response = Get::Hit { value: GetValue::new(vec![]) };
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Many of the docstrings in this file were still referencing the legacy SDK.

@malandis malandis merged commit e8221ab into main May 7, 2024
6 checks passed
@malandis malandis deleted the feat/remove-old-sdk branch May 7, 2024 16:40
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.

2 participants