Skip to content
This repository was archived by the owner on Jun 8, 2019. It is now read-only.

Change Collaborators List to include permissions #116

Closed
wants to merge 2 commits into from

Conversation

zeripath
Copy link

Change the collaborators list to include permissions. Requires go-gitea/gitea#4814.

Copy link
Member

@lafriks lafriks left a comment

Choose a reason for hiding this comment

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

IsCollaborator signature should not be changed

err := c.getParsedResponse("GET",
fmt.Sprintf("/repos/%s/%s/collaborators", user, repo),
nil, nil, &collaborators)
return collaborators, err
}

// IsCollaborator check if a user is a collaborator of a repository
func (c *Client) IsCollaborator(user, repo, collaborator string) (bool, error) {
status, err := c.getStatusCode("GET",
func (c *Client) IsCollaborator(user, repo, collaborator string) (*Permission, error) {
Copy link
Member

Choose a reason for hiding this comment

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

This is breaking change and will break current SDK

Copy link
Author

Choose a reason for hiding this comment

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

Hmm... This was to go with the associated pull-request. Would it better to create a new API end-point to retrieve permissions?

Similarly does the change with reporting the permission in the /repos/{owner}/{repoName}/collaborators break the SDK?

Copy link
Member

Choose a reason for hiding this comment

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

For getting permissions at least new function has to be created even if it would be using same endpoint.

As for type change that is minor breakage as you will still get struct with same properties

Copy link
Author

Choose a reason for hiding this comment

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

Ah ok. I can do that.

Copy link
Author

Choose a reason for hiding this comment

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

Have restored the signature.

@@ -47,6 +47,25 @@ type Repository struct {
Permissions *Permission `json:"permissions,omitempty"`
}

// Collaborator represents a collaborator
// swagger:model
type Collaborator struct {
Copy link
Member

Choose a reason for hiding this comment

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

Why not use

// Collaborator represents a user with collaboration details.
type Collaborator struct {
	*User
	Collaboration *Collaboration
}

Copy link
Member

Choose a reason for hiding this comment

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

ping @zeripath

Copy link
Author

Choose a reason for hiding this comment

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

oh I'd sorta given up on this PR. I don't actually need it with the Sudo approach.

If you think the PR is still worth doing I'll do this.

Copy link
Member

Choose a reason for hiding this comment

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

I think it might be useful to see which permissions collaborators have, but I have no strong opinion, and I was going through PRs marked as stale so I'm ok with this (and linked PR) being closed.

Copy link
Author

Choose a reason for hiding this comment

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

I'll revise, rebase and update

@zeripath zeripath closed this Feb 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants