Support outside collaborators in owners check#197
Support outside collaborators in owners check#197robertbublik wants to merge 1 commit intomszostok:mainfrom
Conversation
robertbublik
changed the title
mszostok
left a comment
mszostok
left a comment
There was a problem hiding this comment.
I have only two small comments, besides that it's a really good job 🚀
internal/check/valid_owner.go
Outdated
| orgName string | ||
| orgTeams []*github.Team | ||
| orgRepoName string | ||
| outsideCollaborators *map[string]struct{} |
There was a problem hiding this comment.
| outsideCollaborators *map[string]struct{} | |
| outsideCollaborators map[string]struct{} |
FYI: We don't use pointers for map. Map is already a reference type, so it does not make sens here. To learn more, see:
https://go.dev/blog/maps
internal/check/valid_owner.go
Outdated
| return newValidateError("User %q is not a member of the organization", name) | ||
| _, isOutsideCollaborator := (*v.outsideCollaborators)[userName] | ||
| if !(isMember || isOutsideCollaborator) { | ||
| return newValidateError("User %q is not an owner of the repository", name) |
There was a problem hiding this comment.
owner in the context of the repository means a bit more 🙂
| return newValidateError("User %q is not an owner of the repository", name) | |
| return newValidateError("The user %q is neither a collaborator nor a member of this repository.", name) |
internal/check/valid_owner.go
Outdated
| opt.Page = resp.NextPage | ||
| } | ||
|
|
||
| v.outsideCollaborators = &map[string]struct{}{} |
There was a problem hiding this comment.
| v.outsideCollaborators = &map[string]struct{}{} | |
| v.outsideCollaborators = map[string]struct{}{} |
robertbublik
force-pushed
the
support-outside-collaborators
branch
from
413a776 to
46700ad
Compare
|
I applied your suggestions! :) Sorry for taking so long, I've been sick for the last two weeks. PS: I guess it would have a made more sense to apply the changes in new commits instead of modifying the original commit. |
mszostok
left a comment
mszostok
left a comment
There was a problem hiding this comment.
Awesome work! Thanks for your contribution 🙇
|
Thank you for approving. :) Is the error in the integration test caused by my changes? |
|
I just opened #222 which I believe would be a better fix for this :) |
3 participants
Description
Changes proposed in this pull request:
outsideCollaboratorsmapinitOutsideCollaboratorsList()function to filloutsideCollaboratorsmapvalidateGitHubUser()whether a given username is an outside collaboratorRelated issue(s)