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.

Sign up for GitHub

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

Jump to bottom

Remove incorrect patch-merge directives. #50257

Merged
merged 1 commit into from
Aug 23, 2017
Merged

Remove incorrect patch-merge directives. #50257

merged 1 commit into from
Aug 23, 2017

Conversation

diegs
Copy link
Contributor

@diegs diegs commented Aug 7, 2017 •
edited
Loading

What this PR does / why we need it:

Directives were misplaced for the following types:

  • MatchExpressions
  • Taints
  • Tolerations

Per the discussion in #46547, we cannot fix these because it would cause backwards-compatibility problems. Instead, remove the incorrect ones so they don't mislead users. This has no impact on behavior.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Special notes for your reviewer:

Takes over from #46547 by @aaronlevy

Release note:

NONE

@k8s-ci-robot
Copy link
Contributor

Hi @diegs. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

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. I understand the commands that are listed here.

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Aug 7, 2017
@k8s-github-robot k8s-github-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. release-note-none Denotes a PR that doesn't merit a release note. labels Aug 7, 2017
@dims
Copy link
Member

dims commented Aug 9, 2017

/ok-to-test

@k8s-ci-robot k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Aug 9, 2017
@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 9, 2017
@zmerlynn zmerlynn removed their assignment Aug 9, 2017
@diegs diegs mentioned this pull request Aug 15, 2017
Closed
@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 16, 2017
@diegs diegs changed the title Fix patch-merge directives. Remove incorrect patch-merge directives. Aug 16, 2017
@diegs
Copy link
Contributor Author

diegs commented Aug 16, 2017

Changed the PR to just remove the incorrect labels, per discussion in #46547.

cc @mengqiy @liggitt

@caesarxuchao
Copy link
Member

lgtm

/assign @LiGgit @mengqiy

@k8s-ci-robot
Copy link
Contributor

@caesarxuchao: GitHub didn't allow me to assign the following users: liggit.

Note that only kubernetes members can be assigned.

In response to this:

lgtm

/assign @LiGgit @mengqiy

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.

@mengqiy
Copy link
Member

mengqiy commented Aug 16, 2017

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 16, 2017
@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 16, 2017
@k8s-github-robot k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 16, 2017
@diegs
Remove incorrect patch-merge directives.
78e1c6f 
Directives were misplaced for the following types:

- MatchExpressions
- Taints
- Tolerations

Per the discussion in kubernetes#46547, we cannot fix these because it would cause
backwards-compatibility problems. Instead, remove the incorrect ones so
they don't mislead users. This has no impact on behavior.
@caesarxuchao
Copy link
Member

/assign @smarterclayton
Could you help approve the change? Thanks.

@diegs
Copy link
Contributor Author

diegs commented Aug 21, 2017

Friendly ping @smarterclayton @caesarxuchao

@smarterclayton
Copy link
Contributor

/approve

@k8s-github-robot k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 23, 2017
@mengqiy
Copy link
Member

mengqiy commented Aug 23, 2017

/lgtm

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: diegs, mengqiy, smarterclayton

Associated issue: 46547

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-github-robot
Copy link

/test all [submit-queue is verifying that this PR is safe to merge]

@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Aug 23, 2017 •
edited
Loading

@diegs: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-e2e-kops-aws 78e1c6f link /test pull-kubernetes-e2e-kops-aws
pull-kubernetes-unit 78e1c6f link /test pull-kubernetes-unit

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 50257, 50247, 50665, 50554, 51077)

@k8s-github-robot k8s-github-robot merged commit baed0e7 into kubernetes:master Aug 23, 2017
@diegs diegs deleted the merge-fix branch August 23, 2017 17:44
This was referenced Jun 7, 2023
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@smarterclayton smarterclayton

@mengqiy mengqiy

@caesarxuchao caesarxuchao

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. release-note-none Denotes a PR that doesn't merit a release note. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@diegs @k8s-ci-robot @dims @caesarxuchao @mengqiy @smarterclayton @k8s-github-robot @zmerlynn