-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Fixing grammatical errors and references #835
Conversation
Hi @animeshsingh. Thanks for your PR. I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
1 similar comment
Hi @animeshsingh. Thanks for your PR. I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
/ok-to-test |
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
Thanks @hongye-sun. Can this be merged? |
/retest |
I think you already have the approval permission. Could you try to approve by yourself? |
I can also approve but just want to verify that the OWNERS file under the ibm-samples directory is working. Let me know if you still need my approval. Thanks. |
/approve |
@animeshsingh: you cannot LGTM your own PR. In response to this:
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. |
1 similar comment
@animeshsingh: you cannot LGTM your own PR. In response to this:
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. |
So @hongye-sun when add the label to approve, seems it still needs an additional approver. So that means the OWNERS file isn`t working? "This pull-request has been approved by: animeshsingh |
/assign @hongye-sun |
@vicaire is the OWNERS file in here not working? Or an OWNER cannot get his own PR in once reviewed by a reviewer? |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: animeshsingh, hongye-sun 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 |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: animeshsingh, hongye-sun 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 |
@animeshsingh, sorry for the delay, I missed your last message. I don't quite understand why the OWNER file doesn't work here. Usually, I can approve my own PR if it's LGTMed by others. |
Thanks @hongye-sun. Who can we discuss this with? Also would love to get in the OWNERS file for 'samples' repo, even if i will just exercise it on ibm-samples? |
Hi @animeshsingh, I think you just need someone else to LGTM your PR. It could be someone from your team or one of us. The comments above from k8-ci-robot list what was missing at each iteration. It may be the same for /approve. You might have to add an approver from your team to the OWNER file. (Note: please make sure not to check-in any binaries) About granting "approve" on the whole "sample" directory, could you suggest instead how we could reorganize the code so that you are not blocked? |
@vicaire LGTM was already added by @hongye-sun - still this does not get in... so the thing missing someone else saying /approve as well? |
@animeshsingh. That's my guess: when you approved, the robot answered that you cannot LGTM your own PR (maybe it should have said that you cannot "approve" your own PR). Note that we are using the Kubernetes bot. Looking at the code could help figure out the exact behavior. I would suggest to have a second approver. If it does not work, we can investigate further. |
This change is