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

GroupGetResult has redundant types #269

Open
skeet70 opened this issue Jun 13, 2022 · 3 comments
Open

GroupGetResult has redundant types #269

skeet70 opened this issue Jun 13, 2022 · 3 comments
Labels
enhancement New feature or request

Comments

@skeet70
Copy link
Member

skeet70 commented Jun 13, 2022

image

Seems like Option<Vec<UserId>> is just Vec<UserId> with extra steps. Is there information communicated by None that isn't communicated by [ ]? If so, we should document that. If not, we should improve the interface so you don't have to check for both None and .len() > 0 when using it.

@skeet70 skeet70 added the enhancement New feature or request label Jun 13, 2022
@skeet70 skeet70 changed the title GroupGetResult redundant types GroupGetResult has redundant types Jun 13, 2022
@giarc3
Copy link
Member

giarc3 commented Jun 13, 2022

I believe the intended distinction is that None comes back from admin_list, member_list, and owner if you aren't a member/admin of the group. But this should definitely be documented better.

@BobWall23
Copy link
Member

Currently, the group permissions include is_admin and is_member - if both are false, you can't get any information about the group admins and members. So this information is somewhat redundant with the none Option for the admin / member lists.

Getting rid of the Option around the vecs gets rid of an extra layer of unwrapping in the consumer, which seems good. We would put this in a v2 ironoxide API feature branch. Not needed any time soon - we can accumulate things in the feature branch until we have enough to release.

@skeet70
Copy link
Member Author

skeet70 commented Jun 15, 2022

@coltfred brought up that we could return an enum type of the GroupMeta and GroupFull types with an impl that lets you use the things that will always be there, and you could match if you need to use something that might not be there (owner or needs_rotation).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants