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.

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

mungegithub: extract hardcoded list of CLA labels into configurable list #3724

Closed
spiffxp opened this issue Jul 26, 2017 · 13 comments
Closed
Labels
lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.

Comments

@spiffxp
Copy link
Member

spiffxp commented Jul 26, 2017

mungegithub currently looks for one of three CLA labels:

  • cla: human-approved
  • cla: yes
  • cncf-cla: yes

a) do we still need to look for all three, or could we just use the cncf-cla label?
b) if not, would it be worth passing in the labels as a configurable list instead of hardcodes?

@spiffxp
Copy link
Member Author

spiffxp commented Jul 26, 2017

@kubernetes/sig-contributor-experience-misc

@cblecker
Copy link
Member

The cla one is the old google bot which I think can go.

The human approved one.. I'm not quite sure what the use case would be for that, or if we permit exceptions to signing the CNCF-CLA. Nothing mentioned here about that: https://github.com/kubernetes/community/blob/master/CLA.md
cc: @sarahnovotny

Either way, it should probably be configurable.

@xiangpengzhao
Copy link
Contributor

xiangpengzhao commented Jul 27, 2017

If some guys want their PR merged when they are still in the progress of signing CLA but not finished, assignees can label cla: human-approved for them.

But this might lead to the PR has both cla: human-approved and cncf-cla: yes finally.

See:
kubernetes/kubernetes#41196 (comment)

@xiangpengzhao
Copy link
Contributor

What if the author fails to sign CLA but the PR is merged with cla: human-approved finally?

@xiangpengzhao
Copy link
Contributor

do we still need to look for all three, or could we just use the cncf-cla label?

IMO, at least, we shouldn't look for cla: yes as it's no longer used. I'm curious about that can a PR be merged if the author only signs old cla but not cncf-cla?

	// Must pass CLA checks
	if gateCLA {
		if !obj.HasLabel(claYesLabel) && !obj.HasLabel(claHumanLabel) && !obj.HasLabel(cncfClaYesLabel) {
			sq.SetMergeStatus(obj, noCLA)
			return false
		}
	}

@foxish
Copy link
Contributor

foxish commented Jul 27, 2017

/cc @yutongz
Is the old CLA label still relevant for you guys?

@yutongz
Copy link
Contributor

yutongz commented Jul 27, 2017

We are not in cncf right now, so still need old one. I agree we can make it configurable. TBH, I like "human-approved" as a hacker to get cla done quickly.

@sarahnovotny
Copy link

@yutongz, can you elaborate? being "in the CNCF" is not a requirement of signing the CNCF CLA. We need to standardize on tooling and the CNCF CLA or we run legal risks with contributions.

@foxish
Copy link
Contributor

foxish commented Jul 27, 2017

@sarahnovotny, Yutong works on Istio that's also using mungegithub, not K8s. The manual override label is still useful I think. Maybe what we need is parameterization here as well, to allow different bots to respect different sets of labels.

@spiffxp
Copy link
Member Author

spiffxp commented Aug 2, 2017

Making the set of labels configurable and not hardcoded seems like the path forward then.

@sarahnovotny re: legal risks, does this mean we should pull support for the cla: human-approved label? PR's that have cla: human-approved but not cncf-cla: yes looked to be not a thing for nearly a year, although we've merged two in the last month

pinging @eparis to ask what happened with kubernetes/kubernetes#49447

@spiffxp spiffxp changed the title mungegithub: why so many cla labels mungegithub: extract hardcoded list of CLA labels into configurable list Aug 2, 2017
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

Prevent issues from auto-closing with an /lifecycle frozen comment.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or @fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 2, 2018
@cblecker
Copy link
Member

cblecker commented Jan 2, 2018

With mungegithub on the way out and tide being the way of the future, I don't think this is relevant anymore. Would you agree, @spiffxp?

@spiffxp
Copy link
Member Author

spiffxp commented Jan 2, 2018

/close
I guess we can close this. I'm guessing we'll want some way of telling a user how to acquire the needed labels, in the same way that submit-queue occasionally updates its status with specific next steps. But that's orthogonal

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.
Projects
None yet
Development

No branches or pull requests

8 participants