-
Notifications
You must be signed in to change notification settings - Fork 153
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
refactor: remove rpc_client::ApiInfo #4280
Conversation
#[derive(Debug, Clone, Copy, PartialOrd, Ord, PartialEq, Eq, Hash)] | ||
enum EndpointStatus { | ||
// RPC method is missing | ||
enum TestSummary { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nonblocking nit: I'm unsure if I like TestSummary
naming, but I can't figure out a better one. This kinda mixes several different concepts together, like actual response errors, check errors and an actual valid status. Perhaps something like TestStatus
would be more intelligible. In my head summary is something that could potentially have several properties, but here we always have only one outcome.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice
Summary of changes
Changes introduced in this pull request:
rpc::Client::default_or_from_env
Url::password
to pass in token information inrpc::Client
ApiInfo
was present, now userpc::Client
orArc<rpc::Client>
crate::rpc_client
rpc_client::RpcRequest
is nowrpc::Request
OneClientInner
toUrlClientInner
EndpointStatus
toTestSummary
static
toconst
for permissions, more efficient permission checking code.Reference issue to close (if applicable)
Closes
Other information and links
Change checklist