Charts: Enable prow merging based on OWNERS files
This issue is to track the status of enabling prow so that those in the OWNERS files who are collaborators on the repo can use prow commands to initiate a merge.
cc: @kubernetes/charts-maintainers
mattfarina
All 21 comments
You will need to change maintainers to approvers in your OWNERS files I think.
cc: @cjwagner
kargakis
on 12 Dec 2017
@kargakis Created a PR for that in #3014
mattfarina
on 12 Dec 2017
Just merged #3014. prow looks excellent BTW!
scottrigby
on 12 Dec 2017
@kubernetes/charts-maintainers We have a detail we need to work out.... do we want to require /lgtm and /approve or just /approve. Any thoughts?
mattfarina
on 12 Dec 2017
I think just /approve
viglesiasce
on 12 Dec 2017
@viglesiasce or just /lgtm? That's an option too.
mattfarina
on 12 Dec 2017
I see that /lgtm adds the label, so that's a functional difference, but is there any semantic difference between the two?
scottrigby
on 12 Dec 2017
What is the standard if any for other projects? Is there a definition of what /lgtm means vs /approve?
viglesiasce
on 12 Dec 2017
Yeah, there is no such thing as just /approve. You either /lgtm and approval is implicit if you are listed as an approver, or you need to /approve explicitly.
kargakis
on 12 Dec 2017
In that case i say just /lgtm
viglesiasce
on 12 Dec 2017
/lgtm is meant to be added by a code reviewer and /approve is meant to be added by a code owner as a high level approval for the change.
kargakis
on 12 Dec 2017
@kargakis does /approve add a /lgtm label? I ask because charts README currently specifies this as part of the review process.
scottrigby
on 12 Dec 2017
Ah gotcha 馃憤 Can anyone run /lgtm command then?
scottrigby
on 12 Dec 2017
@scottrigby /approve should add an approved label.
kargakis
on 12 Dec 2017
/lgtm with implicit approval should also add an approved label.
kargakis
on 12 Dec 2017
Anyone with access to the Kubernetes org can /lgtm and add the lgtm label but the approved label is added based on OWNERS files.
kargakis
on 12 Dec 2017
ok so we should also make sure to update that section of the README. Is there a standard k8s project workflow for these prow commands/labels written up somewhere?
scottrigby
on 12 Dec 2017
See #3008 For an example of using the commands and adding the labels.
I get it enough to do the test-infra config now.
mattfarina
on 12 Dec 2017
kargakis
on 12 Dec 2017
PS, I'm loving looking through these self-documenting prow commands 馃挴
scottrigby
on 12 Dec 2017
This is great :)
cblecker
on 12 Dec 2017
Related issues
jungopro
路
3Comments
vsliouniaev
路
3Comments
kminehart
路
3Comments
richardalberto
路
3Comments
ChrisCooney
路
3Comments
Most helpful comment
PS, I'm loving looking through these self-documenting prow commands 馃挴