Skip to content

Conversation

@michaeldjeffrey
Copy link
Contributor

In every occurence where we have a GAT error, the ability to change the Error is not used. As all of the touched traits are for API Clients, and they all return ClienError, we can reduce the amount of generic constraints we need to type by removing them.

In a case where we do want to return an error, it is a simple process to return an applicable ClientError variant.

In every occurence where we have a GAT error type, the ability to change the Error is not used. As all of the touched traits are for API Clients, and they all return ClienError, we can reduce the amount of generic constraints we need to type by removing them.

In a case where we do want to return an error, it is a simple process to return an applicable ClientError variant.
Copy link
Member

@kurotych kurotych left a comment

Choose a reason for hiding this comment

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

we can reduce the amount of generic constraints

As I understand it, after changing the error from a generic type (like Error: std::fmt::Debug + Send + Sync + 'static) to a specific type like ClientError, we are not reducing the constraints but actually adding a specific one — that the error can only be ClientError.

it is a simple process to return an applicable ClientError variant.

Yes but every time when client want to have a new error, the error must be added to ClientError enum

@michaeldjeffrey
Copy link
Contributor Author

As I understand it, after changing the error from a generic type (like Error: std::fmt::Debug + Send + Sync + 'static) to a specific type like ClientError, we are not reducing the constraints but actually adding a specific one — that the error can only be ClientError.

That's correct, we are reducing the need to specify a restraint that is only ever 1 value. In all cases where the generic was changed from a ClientError it was for the purpose of ignoring errors outright, which can be achieved by simply not using the ClientError.

Yes but every time when client want to have a new error, the error must be added to ClientError enum

This was already the case. There were no implementations using the ability to provide their own error type. And all new errors were added to ClientError. If a good reason to override an error type comes up in the future, we can absolutely implement that use case. But as it stands, these GATs add nothing but visual noise imo.

@kurotych kurotych self-requested a review April 17, 2025 08:26
@michaeldjeffrey michaeldjeffrey merged commit d45d977 into main Apr 17, 2025
53 checks passed
@michaeldjeffrey michaeldjeffrey deleted the mj/remove-gat-errors branch April 17, 2025 16:04
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.

5 participants