Module 1 & 2 #18
Module 1 & 2 #18
Conversation
.github/workflows/ansible-test.yml
Outdated
|
|
||
| - name: Configure integration test run | ||
| env: | ||
| TOKEN: ${{ secrets.GITHUB_TOKEN }} |
There was a problem hiding this comment.
We cannot accept this PR with this integration workflow present/active.
tests/utils/render.sh
Outdated
| eval "echo \"$content\"" | ||
| } | ||
|
|
||
| main "$@" |
| token: | ||
| description: | ||
| - GitHub API token used to retrieve information | ||
| about repositories to which a user has access to |
There was a problem hiding this comment.
to which a user has access to -> to which a user has access
| current_repo_dict["is_template"] = repo.raw_data["is_template"] | ||
|
|
||
| output.append(current_repo_dict) | ||
| if module.check_mode: |
| options: | ||
| token: | ||
| description: | ||
| - GitHub API token used to retrieve information about collaborators in repositories a user has access to |
There was a problem hiding this comment.
repositories a user has access to -> repositories to which a user has access.
| dict_repo.append(collab_output.copy()) | ||
|
|
||
| output[repo] = dict_repo.copy() |
| for collaborator in collaborators: | ||
| collaborator_list.append(collaborator.login) |
There was a problem hiding this comment.
Use a set instead of a list. Using a list results in line 247 having to do a linear search each time it is called.
| status = True | ||
| for repo in repos: | ||
| r = g.get_repo(repo) | ||
| for user in user_to_check.items(): |
There was a problem hiding this comment.
To make this more readable do
for user, permission in user_to_check.items() :
if r.get_colaborator_permission(user) != permission :
....
| r.add_to_collaborators(p, permission=to_add[p]) | ||
|
|
||
|
|
||
| def check_permissions(g, repos, user_to_check): |
There was a problem hiding this comment.
This does not return which user or the repo in which the differences was found. How are playbook authors intended to use check_permissions?
| for p in to_add: | ||
| r.add_to_collaborators(p, permission=to_add[p]) |
There was a problem hiding this comment.
What happens if the user is already a collaborator?
| collaborator_list = [] | ||
| collaborators = r.get_collaborators(affiliation="direct") | ||
| for collaborator in collaborators: | ||
| collaborator_list.append(collaborator.login) |
There was a problem hiding this comment.
None of this is being used. Was the goal to use collaborator_list to control how which users are added?
jacobeicher
force-pushed
the
squashed
branch
from
e132d8b
to
e51bdef
Compare
|
|
There was a problem hiding this comment.
Use a consistent version of python in the GitHub workflows. The ansible test workflow is using 3.6.9. Please use 3.9.x for all workflows.
| for p in to_change: | ||
| if p in collaborator_set: |
There was a problem hiding this comment.
p is a bad variable name. Please change it to something more meaningful.
| def get_collaborators(g, repo): | ||
| output = list() | ||
| collab_output = dict() | ||
| collaborators = g.get_repo(repo).get_collaborators(affiliation="direct") | ||
| for collaborator in collaborators: | ||
| collab_output['login'] = collaborator.login | ||
| collab_output['id'] = collaborator.id | ||
| collab_output['type'] = collaborator.type | ||
| collab_output['site_admin'] = collaborator.site_admin | ||
| permissions = { | ||
| 'triage': collaborator.permissions.triage, | ||
| 'push': collaborator.permissions.push, | ||
| 'pull': collaborator.permissions.pull, | ||
| 'admin': collaborator.permissions.admin | ||
| } | ||
| collab_output['permissions'] = permissions | ||
|
|
||
| output.append(collab_output) | ||
|
|
||
| return output | ||
|
|
There was a problem hiding this comment.
This code does not work. Please fix the bug and add unit tests that would have found the bug.
There was a problem hiding this comment.
output will be populated with objects that have the same values for all fields.
addresses #1 and #2