-
Notifications
You must be signed in to change notification settings - Fork 375
Update KFP Maintainers team #808
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
Update KFP Maintainers team #808
Conversation
70d9a80 to
8bfcac0
Compare
github-orgs/kubeflow/org.yaml
Outdated
| @@ -1085,24 +1085,13 @@ orgs: | |||
| pipelines: | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need this team if we have wg-pipelines-team ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because they maintain the project.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The members are almost identical to the wg-pipeline-leads:
internal-acls/github-orgs/kubeflow/org.yaml
Lines 1205 to 1210 in 8bfcac0
| - chensun | |
| - droctothorpe | |
| - james-jwu | |
| - HumairAK | |
| - mprahl | |
| - zazulam |
Shall we just use the wg-pipelines-leads team to add maintainers and delete the pipelines team?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather delete wg-pipelines-leads
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we first talk with the community what would the future of WGs for Kubeflow ?
We use the WG teams to give write access to the repo, since it requires for many Kubeflow projects to cut releases, for example:
internal-acls/github-orgs/kubeflow/org.yaml
Lines 1230 to 1231 in 8bfcac0
sdk: write trainer: write internal-acls/github-orgs/kubeflow/org.yaml
Line 1191 in 8bfcac0
manifests: write
I think, having two teams will make sense if we:
- add reviewers + approvers + WG leads to the
pipelines-maintainers - add WG leads who require write access to the
wg-pipelines-leads
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to grant write permission access to kubeflow/pipelines repo reviewers as well ?
Usually, write access should be given to the WG leads / sub-project maintainers: https://www.kubeflow.org/docs/about/membership/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This list is all Approvers. I think we should only give access to root level approvers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey folks, please do not give write access to reviewers. Only maintainers should have write access to the KFP repo. Maintainers are the "Approvers" here. The current change looks fine to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@anishasthana @franciscojavierarceo @HumairAK In that case, can we just use the wg-pipelines-team for now and give write access to kubeflow/pipelines repo for this group ?
So we be consistent with other teams (e.g. @kubeflow/wg-training-leads)
In the future, when we discuss the future of WGs, we can update it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, updated.
|
Bumping this. |
Signed-off-by: Anish Asthana <[email protected]>
8bfcac0 to
e7b6e75
Compare
franciscojavierarceo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/approve
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: franciscojavierarceo The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| - HumairAK | ||
| - mprahl | ||
| - zazulam | ||
| privacy: open |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update @anishasthana!
You might also want to give write permission to kubeflow/pipelines repo for @kubeflow/wg-pipeline-leads team
| - HumairAK | ||
| - mprahl | ||
| - zazulam | ||
| privacy: open |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also create PR to keep privacy: closed for this GitHub team @anishasthana ?
It looks like our CI doesn't update the team: https://github.com/orgs/kubeflow/teams/wg-pipeline-leads
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I opened #810
Update KFP Maintainer Teams.
cc @HumairAK @droctothorpe @franciscojavierarceo