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.

Sign up for GitHub

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

Jump to bottom

Set channel and role edit perms to int64 #867

Merged
merged 2 commits into from
Jan 27, 2021
Merged

Set channel and role edit perms to int64 #867

merged 2 commits into from
Jan 27, 2021

Conversation

IntrntSrfr
Copy link
Contributor

Should not the set role/channel permissions methods also take in int64s?

@IntrntSrfr IntrntSrfr changed the title set channel and role edit perms to int64 Set channel and role edit perms to int64 Jan 27, 2021
CarsonHoffman
CarsonHoffman requested changes Jan 27, 2021
View reviewed changes
Copy link
Collaborator

@CarsonHoffman CarsonHoffman left a comment

Choose a reason for hiding this comment

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

Great catch! In addition to the issue you mentioned, we'll also need to switch to serializing these permissions as strings; this is an issue we missed originally and isn't your fault, but would be great to fix in this change!

restapi.go Outdated Show resolved Hide resolved
restapi.go Outdated Show resolved Hide resolved
IntrntSrfr
IntrntSrfr commented Jan 27, 2021
View reviewed changes
Copy link
Contributor Author

@IntrntSrfr IntrntSrfr left a comment

Choose a reason for hiding this comment

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

Done

CarsonHoffman
CarsonHoffman approved these changes Jan 27, 2021
View reviewed changes
Copy link
Collaborator

@CarsonHoffman CarsonHoffman left a comment

Choose a reason for hiding this comment

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

Awesome, LGTM! It appears that API v8 won't reject integers in the JSON payload so this change doesn't need to be immediately deployed, but it'll most likely be rolled out with other smaller changes. Thank you for the contribution! 😃

@CarsonHoffman CarsonHoffman merged commit 5f6bb2d into bwmarrin:master Jan 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CarsonHoffman CarsonHoffman CarsonHoffman approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@IntrntSrfr @CarsonHoffman