github / cleanowners

A GitHub Action to suggest removal of non-organization members from CODEOWNERS files
MIT License
109 stars 7 forks source link

ORGANIZATION is always required #27

Closed timdittler closed 6 months ago

timdittler commented 6 months ago

Contrary to the README, the env var ORGANIZATION is always required. Otherwise the action will fail with


Traceback (most recent call last):
  File "/action/workspace/cleanowners.py", line 206, in <module>
    main()
  File "/action/workspace/cleanowners.py", line [8](https://github.com/Staffbase/infrastructure/actions/runs/8158526447/job/22300640673?pr=5510#step:4:9)[9](https://github.com/Staffbase/infrastructure/actions/runs/8158526447/job/22300640673?pr=5510#step:4:10), in main
    if not github_connection.organization(organization).is_member(username):
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.[12](https://github.com/Staffbase/infrastructure/actions/runs/8158526447/job/22300640673?pr=5510#step:4:13)/site-packages/github3/github.py", line 1688, in organization
    json = self._json(self._get(url), 200)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/github3/models.py", line [16](https://github.com/Staffbase/infrastructure/actions/runs/8158526447/job/22300640673?pr=5510#step:4:17)1, in _json
    raise exceptions.error_for(response)
github3.exceptions.NotFoundError: 404 Not Found
jmeridth commented 6 months ago

@timdittler you cool if I get a PR up for this? or were you planning to go after it?

timdittler commented 6 months ago

@jmeridth Of course. I would be happy if you fixed it!

jmeridth commented 6 months ago

@timdittler reading the code more, I believe we currnetly don't want to make ORGANIZATION required. As the README currently reads, a user can provide a REPOSITORY environment variable instead of ORGANIZATION that includes specific repositories, from potentially multiple organizations. Currently at least one of them is required.

What I'm thinking:

If ORGANIZATION environment variable is provided:

if REPOSITORY environment variable list is provided instead:

note: currently, if both environment variables are used, ORGANIZATION has priority and REPOSITORY is ignored. (future, allow both and parse accordingly)

Gonna try out a PR.