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

devel guide: add words on cherry pick review process #4081

Merged
merged 1 commit into from
Sep 25, 2019
Merged

devel guide: add words on cherry pick review process #4081

merged 1 commit into from
Sep 25, 2019

Conversation

tpepper
Copy link
Member

@tpepper tpepper commented Sep 11, 2019

There has been a fair amount of ambiguity, even deliberately, around
what is a valid candidate for backporting to a stable release branch of
Kubernetes. For some the criteria is simply "critical bug fixes", but
this is also something that lacks definition. This documentation update
attempts to better describe what is expected of and what can be expected
by somebody proposing a cherry-pick on one of our stable release branches.

Signed-off-by: Tim Pepper [email protected]

@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. area/developer-guide Issues or PRs related to the developer guide sig/release Categorizes an issue or PR as relevant to SIG Release. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Sep 11, 2019
@tpepper
Copy link
Member Author

tpepper commented Sep 11, 2019

/cc @xmudrii @PatrickLang @michmike

This was referenced Sep 11, 2019
Closed
Merged
@PatrickLang
Copy link
Contributor

Thanks! I think getting this written down will help resolve a lot of questions

michmike
michmike approved these changes Sep 11, 2019
View reviewed changes
Copy link
Contributor

@michmike michmike left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

thanks for writing this up Tim

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 11, 2019
@tpepper
Copy link
Member Author

tpepper commented Sep 11, 2019 •
edited
Loading

/hold
for broader review.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 11, 2019
@craiglpeters
Copy link
Contributor

/lgtm

xmudrii
xmudrii approved these changes Sep 12, 2019
View reviewed changes
Copy link
Member

@xmudrii xmudrii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@tpepper
Copy link
Member Author

tpepper commented Sep 12, 2019

/wg lts
@kubernetes/wg-lts ya'll have expressed interest in this being more clear. Any review input would be appreciated.

@k8s-ci-robot k8s-ci-robot added the wg/lts Categorizes an issue or PR as relevant to WG LTS. label Sep 12, 2019
@dims
Copy link
Member

dims commented Sep 12, 2019

/cc

@neolit123
Copy link
Member

thanks @tpepper this seems like an improvement over the existing doc.

cpanato
cpanato approved these changes Sep 13, 2019
View reviewed changes
Copy link
Member

@cpanato cpanato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cpanato, michmike, tpepper, xmudrii

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:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

liggitt
liggitt reviewed Sep 13, 2019
View reviewed changes
Copy link
Member

@liggitt liggitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a few nits and suggestions, but this is a big improvement, thanks!

it would be great to reference this doc when running hack/cherry_pick_pull.sh (and maybe include a link to it in the spawned PR's description?

contributors/devel/sig-release/cherry-picks.md Outdated Show resolved Hide resolved
contributors/devel/sig-release/cherry-picks.md Outdated Show resolved Hide resolved
contributors/devel/sig-release/cherry-picks.md Outdated Show resolved Hide resolved
contributors/devel/sig-release/cherry-picks.md Outdated Show resolved Hide resolved
contributors/devel/sig-release/cherry-picks.md Outdated
fork of the Kubernetes project branches. If you were not engaged in a
feature's upstream development and it does not work properly on your
platform, it is not the community's responsibility to support your miss by
backporting feature enablement.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with the sentiment, but the personal targeting of the sentence ("If you were not engaged ... to support your miss ...") seems a touch overbearing. Is there a more neutral way to phrase this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated words.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am curious if folks think this part reads better now.

contributors/devel/sig-release/cherry-picks.md Outdated Show resolved Hide resolved
contributors/devel/sig-release/cherry-picks.md Outdated Show resolved Hide resolved
contributors/devel/sig-release/cherry-picks.md Show resolved Hide resolved
@tpepper tpepper mentioned this pull request Sep 13, 2019
Merged
@tpepper
Copy link
Member Author

tpepper commented Sep 16, 2019 •
edited
Loading

it would be great to reference this doc when running hack/cherry_pick_pull.sh (and maybe include a link to it in the spawned PR's description?

I'll submit a PR for that (opened kubernetes/kubernetes#82758).

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 16, 2019
@tpepper tpepper mentioned this pull request Sep 16, 2019
Merged
liggitt
liggitt reviewed Sep 16, 2019
View reviewed changes
Copy link
Member

@liggitt liggitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one question, one nit. lgtm otherwise.

contributors/devel/sig-release/cherry-picks.md Show resolved Hide resolved
contributors/devel/sig-release/cherry-picks.md Outdated Show resolved Hide resolved
@youngnick
Copy link

I like the specificity, lgtm aside from a very small nit.

@guineveresaenger
Copy link
Contributor

Thank you all so much for this work!

@k8s-ci-robot k8s-ci-robot added the ¯\_(ツ)_/¯ ¯\\\_(ツ)_/¯ label Sep 17, 2019
@dims
Copy link
Member

dims commented Sep 20, 2019

/lgtm 👍

devel guide: add words on cherry pick review process
4cde9de 
There has been a fair amount of ambiguity, even deliberately, around
what is a valid candidate for backporting to a stable release branch of
Kubernetes.  For some the criteria is simply "critical bug fixes", but
this is also something that lacks definition.  This documentation update
attempts to better describe what is expected of and what can be expected
by somebody proposing a cherry-pick on one of our stable release branches.

Signed-off-by: Tim Pepper <[email protected]>
@guineveresaenger
Copy link
Contributor

It's been a week.

/lgtm
/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 25, 2019
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 25, 2019
@k8s-ci-robot k8s-ci-robot merged commit bd3b936 into kubernetes:master Sep 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PatrickLang PatrickLang PatrickLang left review comments

@youngnick youngnick youngnick left review comments

@liggitt liggitt liggitt left review comments

@neolit123 neolit123 neolit123 left review comments

@xmudrii xmudrii xmudrii approved these changes

@michmike michmike michmike approved these changes

@cpanato cpanato cpanato approved these changes

@guineveresaenger guineveresaenger Awaiting requested review from guineveresaenger

@justaugustus justaugustus Awaiting requested review from justaugustus

@dims dims Awaiting requested review from dims

Assignees

@cpanato cpanato

@michmike michmike

@craiglpeters craiglpeters

@guineveresaenger guineveresaenger

@xmudrii xmudrii

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/developer-guide Issues or PRs related to the developer guide 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. sig/release Categorizes an issue or PR as relevant to SIG Release. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. wg/lts Categorizes an issue or PR as relevant to WG LTS. ¯\_(ツ)_/¯ ¯\\\_(ツ)_/¯
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.