search

knative / client-contrib

Community contributed `kn` plugins.
Apache License 2.0
10 stars 23 forks source link

First setup for an integrated CI run #12

Closed rhuss closed 4 years ago

rhuss commented 4 years ago

/retest

rhuss commented 4 years ago

/hold

rhuss commented 4 years ago
  • Do you like to add a OWNERS file under each plugin to specify the reviewers/approvers to this specific plugin, just like event-contrib does ?

Yes, that's a good idea.

  • What's the reason for copying the code of test-infra other than using dependency like client repo ? I see you have a script update-test-infra.sh. Do you plan to call this script for every update of test-infra ?

Exactly. The main reason is, that I want to share the same test-infra scripts across all plugins as the CI is the same for every plugin. However, the top-level directory is not a go module, so we can't reuse the existing dependency mechanism as we do in client. So I've written that shell script which we should call for any update and then commit the files in the test-infra top-level directory (a bit the same like updating the go.mod file when we need to update).

FYI, I'm still working on the test integration but I think I'm close to be finished.

knative-prow-robot commented 4 years ago

@rhuss: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
pull-knative-client-contrib-go-coverage c62617e88c52614d54b0036296a89e14e5956bc7 link /test pull-knative-client-contrib-go-coverage

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
rhuss commented 4 years ago

/unhold

knative-prow-robot commented 4 years ago

@navidshaikh: changing LGTM is restricted to collaborators

In response to [this](https://github.com/knative/client-contrib/pull/12#pullrequestreview-388408448): >/lgtm Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
knative-prow-robot commented 4 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: navidshaikh, rhuss

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: - ~~[OWNERS](https://github.com/knative/client-contrib/blob/master/OWNERS)~~ [rhuss] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
navidshaikh commented 4 years ago

/lgtm

knative-prow-robot commented 4 years ago

@navidshaikh: changing LGTM is restricted to collaborators

In response to [this](https://github.com/knative/client-contrib/pull/12#issuecomment-609897803): >/lgtm Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
rhuss commented 4 years ago

@chizhg could you please have a look, we can't merge this PR although @navidshaikh is mentioned as approver:

@navidshaikh: changing LGTM is restricted to collaborators

What need to be changed to allow the OWNER file to be picked up ?

chizhg commented 4 years ago

/lgtm

chizhg commented 4 years ago

/lgtm cancel

chizhg commented 4 years ago

@chizhg could you please have a look, we can't merge this PR although @navidshaikh is mentioned as approver:

@navidshaikh: changing LGTM is restricted to collaborators

What need to be changed to allow the OWNER file to be picked up ?

This looks weird, even I can lgtm.. The logic should be anyone that has joined Knative will be able to lgtm, but I believe @navidshaikh has already joined (?)

rhuss commented 4 years ago

@chizhg Yes, @navidshaikh is approver on the client repo, too.

It could be that, I confused Prow a bit, because I added an new OWNERS in a sub directory and adapted the original one (to remove cppforlife who is no longer active with Knative) in this PR, but I reverted this again (but even then, @navidshaikh couldn't do it).

But then on the other hand, /lgtm work for any Knative member, right ?

Let me try tomorrow again, when some other folks are online again.

thanks, @chizhg ...

navidshaikh commented 4 years ago

/lgtm

knative-prow-robot commented 4 years ago

@navidshaikh: changing LGTM is restricted to collaborators

In response to [this](https://github.com/knative/client-contrib/pull/12#issuecomment-610207744): >/lgtm Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
dsimansk commented 4 years ago

/lgtm

rhuss commented 4 years ago

@chizhg @chaodaiG Next issue: There is no CLA bot running against this repo to attach the "cla = yes" label. Could please add this bot to watch this repo ? Thanks !

rhuss commented 4 years ago

/retest

googlebot commented 4 years ago

:frowning_face: Sorry, but only Googlers may change the label cla: yes.

rhuss commented 4 years ago

/retest cla/google

dsimansk commented 4 years ago

/check-cla

rhuss commented 4 years ago

@googlebot I consent.