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.

Sign up for GitHub

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

Jump to bottom

Revert 'Revert migrate maintainers to members for k-incubator' #592

Closed
wants to merge 1 commit into from
Closed

Revert 'Revert migrate maintainers to members for k-incubator' #592

wants to merge 1 commit into from

Conversation

nikhita
Copy link
Member

@nikhita nikhita commented Mar 11, 2019

Reverts #591

#591 fixed peribolos and it's now green...but we still need to migrate maintainers to members for k-incubator.

/cc @spiffxp @mrbobbytables @justaugustus @rlenferink @idealhack
/assign @spiffxp @fejta
/hold

@nikhita
Revert "Merge pull request kubernetes#591 from nikhita/revert-migrate…
d5d9b1d 
…-maintainers-for-k-incubator"

This reverts commit 25767f0, reversing
changes made to 43666b8.
@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 11, 2019
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 11, 2019
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Mar 11, 2019
@rlenferink
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 11, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nikhita, rlenferink

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@fejta
Copy link
Contributor

fejta commented Mar 11, 2019

If reverting #591 is what fixed peribolos, won't adding it back break peribolos again?

@justaugustus
Copy link
Member

If reverting #591 is what fixed peribolos, won't adding it back break peribolos again?

Yeah, I'm also curious as to what's changed.

@nikhita
Copy link
Member Author

nikhita commented Mar 15, 2019

Yeah, I am not entirely sure why we were hitting a 403 in the first place. Note that we weren't hitting the token limit because this error is displayed after checking if we've exceeded the rate limit.

It also looked like we were getting tripped on the same team each time. I was thinking that we could make changes to all teams, but except the problematic team and try syncing the problematic team in another PR. Any other suggestions?

@justaugustus
Copy link
Member

Ahhh, the offending team is not in this PR? If that's the case, then SGTM!

@rlenferink
Copy link
Member

Trying to remove the problematic team and see how it behaves may be good to get started with indeed.

@nikhita can you update your PR to exclude changes to the maintainers-rktlet team, as that's the team failing?

@nikhita
Copy link
Member Author

nikhita commented Mar 18, 2019

(I'm semi-afk today, will do that tomorrow 😬 )

@rlenferink
Copy link
Member

np, take your time 🙂

@nikhita nikhita mentioned this pull request Mar 21, 2019
Closed
@fejta
Copy link
Contributor

fejta commented Mar 22, 2019

This still alive?

@k8s-ci-robot
Copy link
Contributor

@nikhita: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 31, 2019
@spiffxp
Copy link
Member

spiffxp commented Apr 19, 2019

needs rebase

@cblecker cblecker mentioned this pull request May 29, 2019
Merged
@cblecker
Copy link
Member

Closing in favour of #849

@cblecker cblecker closed this May 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rlenferink rlenferink rlenferink approved these changes

@idealhack idealhack Awaiting requested review from idealhack

@justaugustus justaugustus Awaiting requested review from justaugustus

@mrbobbytables mrbobbytables Awaiting requested review from mrbobbytables

@spiffxp spiffxp Awaiting requested review from spiffxp

Assignees

@spiffxp spiffxp

@fejta fejta

@rlenferink rlenferink

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@nikhita @rlenferink @k8s-ci-robot @fejta @justaugustus @spiffxp @cblecker