initial support for gitea #148
Conversation
Signed-off-by: Malar Kannan <[email protected]>
| return nil, fmt.Errorf("no files added") | ||
| } | ||
|
|
||
| // TODO: fix workaround |
There was a problem hiding this comment.
awaiting upstream fix go-gitea/gitea#14619
gitea/giteaclient.go
Outdated
| return c.c | ||
| } | ||
|
|
||
| func (c *giteaClientImpl) GetOrg(ctx context.Context, orgName string) (*gitea.Organization, error) { |
There was a problem hiding this comment.
Please add comments to exported objects.
@souleb Thanks for the review. This is more of a learning exercise for me, so not in any hurry. Do note that this is my first time ever attempt at writing go code. so feel free to review it under a microscope 🙂. I will go over your review comments and revert with more changes, soon. |
2. underscored unused args 3. commented exported api 4. enabled custom domain auth_test cases 5. typo, unuse var fixes 6. pagination support and bugfixes Signed-off-by: Malar Kannan <[email protected]>
| // operating on the gitea structs. TODO: Verify pagination is implemented for all List* methods, | ||
| // all returned objects are validated, and HTTP errors are handled/wrapped using handleHTTPError. | ||
| // This interface is also fakeable, in order to unit-test the client. | ||
| type giteaClient interface { |
There was a problem hiding this comment.
We are lucky no one rely on this interface yet. Normally I wouldn't advise for such a large interface, in GO you should try to keep interfaces small, and composable. I get it you want to keep existing code structure, nevertheless, please modify the functions structures as needed (e.g. some of them don't need context.Context parameters)
| teamFound := false | ||
| for _, team := range teams { | ||
| if team.Name != teamName { | ||
| continue | ||
| } | ||
| teamFound = true | ||
| users, _, err := c.c.ListTeamMembers(team.ID, gitea.ListTeamMembersOptions{}) | ||
| if err != nil { | ||
| continue | ||
| } | ||
| apiObjs = append(apiObjs, users...) | ||
| } | ||
| if teamFound { | ||
| return apiObjs, nil | ||
| } | ||
| return nil, gitprovider.ErrNotFound |
There was a problem hiding this comment.
As we expect only one team with the same, we should exit early after retrieving the team. We could do something like:
for _, team := range teams {
if team.Name == teamName {
users, _, err := c.c.ListTeamMembers(team.ID, gitea.ListTeamMembersOptions{})
if err != nil {
return nil, fmt.Errorf("failed to list team %s members: %w", teamName, err)
}
return users, nil
}
return nil, gitprovider.ErrNotFound
| // Client returns the underlying *gitea.Client | ||
| Client() *gitea.Client | ||
|
|
||
| // GetOrg is a wrapper for "GET /orgs/{org}". |
There was a problem hiding this comment.
The api endpoint is implementation specific in my opinion, so I would see this as a comment for the implementing function.
| if err != nil { | ||
| return handleHTTPError(resp, err) | ||
| } | ||
| if resp == nil { |
| if resp == nil { | ||
| return nil | ||
| } | ||
| opts.Page += 1 |
There was a problem hiding this comment.
I would increment by resp.NextPage, because it is not guaranteed that each response contains only 1 page.
There was a problem hiding this comment.
what do you mean? there is no resp.NextPage in the gitea resp object.
| return &apiObj.Permission, nil | ||
| } | ||
|
|
||
| func (c *giteaClientImpl) ListRepoTeams(ctx context.Context, orgName, repo string) ([]*gitea.Team, error) { |
There was a problem hiding this comment.
Could we use GetRepoTeams() here?
|
@malarinv Are you still interested in updating the PR to get it merged? |
Signed-off-by: Malar Kannan [email protected]
Description
Enable support for gitea provider
fixes #147
Test results