Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: filter repos by team ownership #242

Merged
merged 5 commits into from
Oct 5, 2024
Merged

feat: filter repos by team ownership #242

merged 5 commits into from
Oct 5, 2024

Conversation

zkoppert
Copy link
Member

@zkoppert zkoppert commented Oct 4, 2024

Pull Request

fixes #74

This pull request adds a feature to support specifying a team in addition to an organization. This enables users to run the action on all the repositories owned by a team instead of having to keep that list up to date with the team ownership data already in GitHub. The most important changes include updating the README.md to reflect the new TEAM_NAME parameter, modifying the env.py and evergreen.py files to handle the new parameter, and adding tests in test_env.py to ensure the new functionality works as expected.

Proposed Changes

Documentation Updates:

  • README.md: Added TEAM_NAME parameter to describe how to configure the action for a specific team within an organization. [1] [2] [3]

Code Enhancements:

  • env.py: Updated get_env_vars function to include team_name parameter and added validation to ensure TEAM_NAME is not used with ORGANIZATION or REPOSITORY. [1] [2] [3] [4]
  • evergreen.py: Modified the main function and get_repos_iterator to handle repositories based on the TEAM_NAME parameter. [1] [2] [3] [4]

Testing:

  • test_env.py: Added new tests to verify the correct behavior when TEAM_NAME is set and to ensure errors are raised when TEAM_NAME is used without ORGANIZATION or with REPOSITORY. [1] [2] [3] [4] [5] [6]

Readiness Checklist

Author/Contributor

  • If documentation is needed for this change, has that been included in this pull request
  • run make lint and fix any issues that you have introduced
  • run make test and ensure you have test coverage for the lines you are introducing
  • If publishing new data to the public (scorecards, security scan results, code quality results, live dashboards, etc.), please request review from @jeffrey-luszcz

Reviewer

  • Label as either fix, documentation, enhancement, infrastructure, maintenance or breaking

@zkoppert zkoppert self-assigned this Oct 4, 2024
@zkoppert zkoppert marked this pull request as ready for review October 4, 2024 19:22
env.py Outdated
raise ValueError(
"REPOSITORY environment variable was not set correctly. Please set it to a comma separated list of repositories in the format org/repo"
"TEAM_NAME environment variable cannot be used with ORGANIZATION or REPOSITORY"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought organization was required when using TEAM_NAME?

Also maybe add check if TEAM-NAME is used then ORGANIZATION is also set?

Copy link
Member Author

@zkoppert zkoppert Oct 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! I totally got that messed up on the wording. Fixing...

I originally had in here to check if TEAM-NAME is used then ORGANIZATION is also set but the code was unreachable because we already check for ORGANIZATION or REPOSITORY, and then for TEAM_NAME and not REPOSITORY, so by deduction we've confirmed ORGANIZATION is present. By unreachable code, I mean to say I couldn't write a test that hit it because of the check for org first. If you think I should rewrite the order of the checks so its clear that case is covered, I'm happy to do that. At the time I just elected to remove unreachable code but that isn't as readable/clear.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point re: unreachable code. I'm a fan of explicit over implicit. Not a blocker though.

env.py Outdated Show resolved Hide resolved
@zkoppert zkoppert merged commit 5568ead into main Oct 5, 2024
25 checks passed
@zkoppert zkoppert deleted the filter-by-team branch October 5, 2024 06:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support filtering by team
2 participants