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

Remove the various client traits #108

Merged
merged 1 commit into from
Dec 1, 2020
Merged

Remove the various client traits #108

merged 1 commit into from
Dec 1, 2020

Conversation

rylev
Copy link
Contributor

@rylev rylev commented Nov 30, 2020

This is the first in (I hope) many PRs to remove most of the traits we use internally in Cosmos.

This removes all traits which seek to require client getter methods. I don't see any advantage to these being a part of a trait instead of being on the struct itself. I believe the thought originally behind this was consistent naming, but I think it's easier to manually keep track of that rather than relying on traits (after all, the user could just choose to not implement the trait at all).

I believe we should remove all traits which exist only to enforce naming conventions. I believe this will most likely result in removing the entire traits.rs file in Cosmos DB.

@rylev rylev added the Cosmos The azure_cosmos crate label Nov 30, 2020
Copy link
Contributor

@MindFlavor MindFlavor left a comment

Choose a reason for hiding this comment

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

I like it even though it removes the documentation property of the traits for the sake of some typing. For example by enumerating the implementers of TriggerClientRequired one can find all the functions pertaining to triggers.

It's probably best to keep the code simpler 👍 !

@rylev
Copy link
Contributor Author

rylev commented Dec 1, 2020

even though it removes the documentation property of the traits for the sake of some typing.

I'm not so sure I see that as being a very common need. When will people want to see all the types that implement that method?

The most important change that this leads to is it improves documentation. Below is a screenshot of the docs. Notice that it's easy to see the permission_client method in the docs now. The methods defined on the traits are only viewable with the docs expanded and even they they are surrounded by the noise of the trait definitions.

image

@rylev rylev merged commit 70aef03 into Azure:master Dec 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cosmos The azure_cosmos crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants