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: use team id as maintainers #173

Conversation

IronCore864
Copy link
Contributor

@IronCore864 IronCore864 commented Sep 5, 2024

This PR changes the previous "owners", a list of GitHub user IDs, to "maintainers", a string of GitHub team ID. Assigning issues to a team is not supported, so an extra feature is added where user IDs are fetched from team ID.

More background:

Per discussions in this issue (see comment here and here) and an email thread, we have agreed to the followings:

  • Use "maintainers" instead of "owners", because "owners" indicates ownership while the interface is meant to be "owned" by a group around it.
  • Put a GitHub team ID as "maintainers", instead of a list of users. This eliminates the overhead of maintaining the list of user IDs. The purpose of this field is to assign issues to users when a certain interface test fails; although it's not supported to assign an issue to a team, this can be worked out in the charm-relation-interfaces repo's run_matrix.py script.

This PR to charm-relation-interfaces reflects the above changes, see the other part in the pytest-interface-tester repo. Before merging into the main branch, it can’t be tested using GitHub Actions, but I’ve tested it in a forked repo in my own org, see here.

@IronCore864 IronCore864 marked this pull request as ready for review September 25, 2024 03:44
@IronCore864
Copy link
Contributor Author

IronCore864 commented Sep 25, 2024

Now that the PR for pytest-interface-tester is merged, this PR can be merged to use the latest pytest-interface-tester, v3.1.1, that has the "maintainer" feature.

Note that this PR needs to be merged first in order for the checks to pass.

Copy link
Collaborator

@benhoyt benhoyt left a comment

Choose a reason for hiding this comment

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

Looks good. One code style suggestion.

run_matrix.py Outdated Show resolved Hide resolved
@IronCore864
Copy link
Contributor Author

IronCore864 commented Oct 10, 2024

This PR can be merged since pytest-interface-tester 3.2.0 has been released with the maintainer feature, and the tests have been fixed, see passed checks below.

Squash all commits, sign, and merge now.

@IronCore864 IronCore864 removed the request for review from simskij October 10, 2024 11:23
@IronCore864 IronCore864 force-pushed the change-owners-to-maintainers-and-support-using-team-id branch from 0683e0d to 1a45a6c Compare October 10, 2024 11:43
@IronCore864 IronCore864 force-pushed the change-owners-to-maintainers-and-support-using-team-id branch from 1a45a6c to c5d9c12 Compare October 10, 2024 11:44
@IronCore864 IronCore864 merged commit fde6800 into canonical:main Oct 10, 2024
6 checks passed
@IronCore864 IronCore864 deleted the change-owners-to-maintainers-and-support-using-team-id branch October 10, 2024 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants