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

Plural name in client role management method #248

Closed
GabriFila opened this issue Dec 18, 2020 · 1 comment · Fixed by #361
Closed

Plural name in client role management method #248

GabriFila opened this issue Dec 18, 2020 · 1 comment · Fixed by #361
Labels
backward incompatible Backward Incompatible Changes enhancement New feature or request

Comments

@GabriFila
Copy link

Is your feature request related to a problem? Please describe.
This might be nitpicking but I would like to point it out anyway. I believe that the name for the methods

  • AddClientRoleToUser
  • AddClientRoleToGroup
  • DeleteClientRoleFromUser
    should be
  • AddClientRolesToUser
  • AddClientRolesToGroup
  • DeleteClientRolesFromUser

because they take as argument a slice of Role structs

Describe the solution you'd like
The change consists of using the plural in the name, like the method GetClientRolesByUserID

I'd be happy to fix it

Hope this was helpful. Thank you!

@Nerzal
Copy link
Owner

Nerzal commented Mar 27, 2021

Hi,
i understand that and agree, we can do this, but this will be a breaking change, so we'll only merge it, if we need to introduce other breaking changes :)

@Nerzal Nerzal added backward incompatible Backward Incompatible Changes enhancement New feature or request labels Mar 27, 2021
Nerzal pushed a commit that referenced this issue Aug 4, 2022
@Nerzal Nerzal closed this as completed in 18f0f3d Oct 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backward incompatible Backward Incompatible Changes enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants