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

Replace OverwriteId type with Either RoleId UserId #121

Merged
merged 2 commits into from May 23, 2022

Conversation

yutotakano
Copy link
Member

@yutotakano yutotakano commented May 22, 2022

Summary

This pull request replaces OverwriteId with Either RoleId UserId, and removes the type flag from surrounding ADTs.

API Changes

  • OverwriteId is removed.
  • Argument types for EditChannelPermissions and DeleteChannelPermission have changed to use Either.
  • ChannelPermissionsOptsType is removed since Either is enough to replace this.
  • The Overwrite ADT no longer has a overwriteType flag.
  • The ChannelPermissionsOpts ADT no longer has a channelPermissionsOptsType flag.

Motivation

I was trying to invoke a request to EditChannelPermissions. This requires an OverwriteId as one of its arguments, which is either a role or user id (specified by a flag inside the ADT). The official docs for this are here.

Before the recent ID overhaul, I could pass either UserId or RoleId with no problem since it was all the same type internally. However since the types are opaque now, I have to unwrap the Id and wrap it once again as an OverwriteId to use it. This is quite cumbersome.

The current code is also not making full use of the types Haskell offers, since the Either type can solve the above problem, and actually even more by getting rid of the flag parameters!

@aquarial
Copy link
Collaborator

Very neat PR and explanation. Trimming the code types for more type safety because the Id system got more safe.

👍

@aquarial aquarial merged commit 39e54cb into discord-haskell:master May 23, 2022
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.

None yet

2 participants