github / cleanowners

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

feat: allow github app authentication #30

Closed jmeridth closed 7 months ago

jmeridth commented 7 months ago

Pull Request

Proposed Changes

Readiness Checklist

Author/Contributor

Local Testing and Linting

Testing

```bash ➜ make test pytest -v --cov=. --cov-config=.coveragerc --cov-fail-under=80 --cov-report term-missing ====================================================== test session starts ======================================================= platform darwin -- Python 3.11.8, pytest-8.1.1, pluggy-1.4.0 -- /Users/jmeridth/code/cleanowners/.venv/bin/python3.11 cachedir: .pytest_cache rootdir: /Users/jmeridth/code/cleanowners plugins: cov-4.1.0 collected 15 items test_auth.py::TestAuth::test_auth_to_github_with_ghe PASSED [ 6%] test_auth.py::TestAuth::test_auth_to_github_with_github_app PASSED [ 13%] test_auth.py::TestAuth::test_auth_to_github_with_token PASSED [ 20%] test_auth.py::TestAuth::test_auth_to_github_without_token PASSED [ 26%] test_cleanowners.py::TestCommitChanges::test_commit_changes PASSED [ 33%] test_cleanowners.py::TestGetUsernamesFromCodeowners::test_get_usernames_from_codeowners PASSED [ 40%] test_cleanowners.py::TestGetReposIterator::test_get_repos_iterator_with_organization PASSED [ 46%] test_cleanowners.py::TestGetReposIterator::test_get_repos_iterator_with_repository_list PASSED [ 53%] test_env.py::TestEnv::test_get_env_vars_missing_org_or_repo PASSED [ 60%] test_env.py::TestEnv::test_get_env_vars_missing_token PASSED [ 66%] test_env.py::TestEnv::test_get_env_vars_optional_values PASSED [ 73%] test_env.py::TestEnv::test_get_env_vars_with_github_app_and_repos PASSED [ 80%] test_env.py::TestEnv::test_get_env_vars_with_org PASSED [ 86%] test_env.py::TestEnv::test_get_env_vars_with_repos_no_dry_run PASSED [ 93%] test_env.py::TestEnv::test_get_env_vars_with_token_and_repos PASSED [100%] ---------- coverage: platform darwin, python 3.11.8-final-0 ---------- Name Stmts Miss Cover Missing ---------------------------------------------- auth.py 14 1 93% 41 cleanowners.py 36 0 100% env.py 60 10 83% 25-26, 68-69, 80, 96, 122, 131, 141, 150 ---------------------------------------------- TOTAL 110 11 90% Required test coverage of 80% reached. Total coverage: 90.00% ```

Linting

```bash ➜ make lint # stop the build if there are Python syntax errors or undefined names flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics --exclude=venv,.venv,.git,__pycache__ 0 # exit-zero treats all errors as warnings. The GitHub editor is 127 chars wide flake8 . --count --exit-zero --max-complexity=100 --max-line-length=150 --statistics --exclude=venv,.venv,.git,__pycache__ 0 pylint --rcfile=.pylintrc --fail-under=9.0 *.py ```

Reviewer

jmeridth commented 7 months ago

Unable to test superlinter locally due to https://github.com/super-linter/super-linter/pull/5320. I'm watching that PR. So I will push fixes if any issues superlinter GitHub Action finds 😄

jmeridth commented 7 months ago

~Working through the linting errors. Handled most linting errors. Wondering if those fix the last 3 I was seeing.~ Had to change the type of the token as os.getenv returns either a string or None (str | None).

jmeridth commented 7 months ago

Going to run black formatter locally before open PRs. Thanks @zkoppert for handling.

zkoppert commented 7 months ago

Taking a deeper look at this today!

jmeridth commented 7 months ago

Clearer documentation ftw. Thank you @zkoppert