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

Enable milestone munger for PRs #5496

Merged
merged 1 commit into from
Nov 14, 2017

Conversation

marun
Copy link
Contributor

@marun marun commented Nov 14, 2017

As per a request from @enisoc to have PRs in the milestone maintained by the munger ahead of code slush/freeze. The majority of the changes are to ensure the correct object type (issue/pr) is referenced in notifications. In addition to minor test updates, I've manually tested the munger against a personal repository.

cc: @jberkus

@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Nov 14, 2017
@enisoc
Copy link
Member

enisoc commented Nov 14, 2017

/assign @apelisse

Antoine, could you take a look? I'd like to enable this as soon as possible, so we'll be ready for code slush on Monday.

@apelisse
Copy link
Member

yeah, looks good to me.
/lgtm

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: apelisse, marun

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

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 14, 2017
@k8s-ci-robot k8s-ci-robot merged commit 357f70b into kubernetes:master Nov 14, 2017
@enisoc
Copy link
Member

enisoc commented Nov 14, 2017

@cjwagner Are you the one who deploys munger updates?

@cjwagner
Copy link
Member

Yeah I'll update the k/k deployment.

@xiangpengzhao
Copy link
Contributor

@marun kubernetes/kubernetes#55812 has both status/approved-for-milestone (added manually) and milestone/incomplete-labels. Is this status expected?

@enisoc
Copy link
Member

enisoc commented Nov 17, 2017

@xiangpengzhao That one needs a priority/* label: kubernetes/kubernetes#55812 (comment)

@xiangpengzhao
Copy link
Contributor

xiangpengzhao commented Nov 17, 2017

@enisoc yeah I saw that comment. What I was thinking of is the milestone workflow (IIRC):

  • PR is marked in the milestone, then [UPDATE: add this bullet]
  • PR has milestone/incomplete-labels, then
  • PR has all of priority/*, kind/*, sig/*, then
  • PR removes milestone/incomplete-labels and adds milestone/complete-labels, then
  • PR is approved for milestone, it gets status/approved-for-milestone.

So I wonder if it's a correct status that a PR has both status/approved-for-milestone and milestone/incomplete-labels like the linked PR does. If not, the bot should remove the manually added status/approved-for-milestone until all the expected labels are labeled.

@enisoc
Copy link
Member

enisoc commented Nov 17, 2017

I don't think we should remove status/approved-for-milestone in this case. That's often added by someone who's in milestone-maintainers doing a drive-by to approve an issue. We shouldn't require that person to also set the other labels before approving, and we shouldn't make them have to come back again because status/approved-for-milestone didn't stick.

@xiangpengzhao
Copy link
Contributor

@enisoc agreed :)

@xiangpengzhao
Copy link
Contributor

another thing, after the final label status/approved-for-milestone is added, do we still need the last comment? See: kubernetes/kubernetes#55803 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants