- Notifications
You must be signed in to change notification settings - Fork673
add group members all#599
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
@hans-d Thanks for your contribution! Could you please also add some tests and add this endpoint to the documentation? Thanks 😃 |
| pass | ||
| class GroupMemberAllManager(ListMixin, RESTManager): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I understand your goal here, but I think it would make more sense to add anall() method to theGroupMemberManager class. TheRunnerManager class has the same logic. From a REST point of viewall is an action applied to the resource at path/groups/:id:/members (GroupMember).
Does that make sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Makes sense, hadn't seen that variation yet. Will try to change it like that soonish
Hi@hans-d Is it OK with you if I take over this MR? Thanks |
Fine by me |
TobiX commentedNov 7, 2018
This should probably be implemented in the same way for project members... |
I pushed#642 which will add support for the method for groups and projects. Closing this one. |
fixes#589:
gitlab group-member-all list --group-id $id