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

Branch Management handbook Update for v1.16 #666

Merged

Conversation

Bubblemelon
Copy link
Contributor

@Bubblemelon Bubblemelon commented Jun 14, 2019

Contains updates for the upcoming release, added and updated sections, fixed phrasing, fixed formatting and etc.

View full document

To: @idealhack
As future lead, please don't hesitate to let me know if you have any concerns with the changes.

This PR also addresses #659, see section Configure Merge Automation for the added task from test-infra.

CC: @imkin for approval on the above section.

To: @vivektaparia @chrisz100 @SlickNik
Since you all will move on as shadows for the next cycle, please help peer-review/fact-check and let me or @idealhack know if there's any concerns 😃

CC: @tpepper
Just FYI.
I welcome and appreciate your input wherever necessary.

@Bubblemelon Bubblemelon requested review from idealhack and imkin June 14, 2019 17:59
@k8s-ci-robot k8s-ci-robot requested review from dougm and hoegaarden June 14, 2019 17:59
@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. approved Indicates a PR has been approved by an approver from all required OWNERS files. area/release-team Issues or PRs related to the release-team subproject sig/release Categorizes an issue or PR as relevant to SIG Release. labels Jun 14, 2019
@justaugustus
Copy link
Member

/cc

@imkin
Copy link

imkin commented Jun 14, 2019

Hey @Bubblemelon Thanks for working on this.

Please add sections for https://github.com/kubernetes/sig-release/tree/master/release-team/role-handbooks/test-infra#create-cipresubmit-jobs-for-the-new-release

Please add a separate page which might link to key resource https://github.com/kubernetes/sig-release/tree/master/release-team/role-handbooks/test-infra#useful-links

Please remove references to test-infra team and add references to "test-infra-oncall", sig-testing, testing-ops channel.

Add to the requirements section for lead and shadow the added load of doing these tasks.

@imkin
Copy link

imkin commented Jun 14, 2019

/hold

@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 Jun 14, 2019
@Bubblemelon
Copy link
Contributor Author

Re:

Please add sections for https://github.com/kubernetes/sig-release/tree/master/release-team/role-handbooks/test-infra#create-cipresubmit-jobs-for-the-new-release

Wasn't this the section that CI Signal was taking over but @jberkus opposed. So is Bug Triage taking over? And is there a link to that discussion somewhere?

@Bubblemelon
Copy link
Contributor Author

Re:

Please add a separate page which might link to key resource https://github.com/kubernetes/sig-release/tree/master/release-team/role-handbooks/test-infra#useful-links

Would it be better to take content from the Useful Links section and elaborate under Viewing test results in k/test-infra ? Since some are duplicates in "View test results" already?

@imkin
Copy link

imkin commented Jun 14, 2019

@Bubblemelon
The proposal was I would initially suggest that config editing should fall to the branch manager as part of the branch cut process. And this stood well through the discussions. Forking on new jobs is tied to creation of the new release branch. It should involve CI-signal to review the dashboards in the PR.

@imkin
Copy link

imkin commented Jun 14, 2019

Re:

Would it be better to take content from the Useful Links section and elaborate under Viewing test results in k/test-infra ? Since some are duplicates in "View test results" already?

These links were constructed over time to have a "one place" to view the important/useful links for anyone performing these duties. The test-infra documentation is too detailed and the links are spread across multiple mark-downs. One should always read through test-infra documentation but to get a quick set of links this one helped us.

@chrisz100
Copy link

/cc

@k8s-ci-robot k8s-ci-robot requested a review from chrisz100 June 15, 2019 18:38
@justaugustus
Copy link
Member

The PR is already on hold, but just mentioning here that I'd like to thoroughly review this before it goes in.

First things first...

@Bubblemelon -- can you squash these commits?
I'd feel a little differently if it was a code change, but since it's markdown, let's just tighten up the git history and add a great body to the commit message:

@tpepper
Copy link
Member

tpepper commented Jun 17, 2019

I too want to give this a thorough read/comment...expect to in the next couple days.

Copy link
Member

@idealhack idealhack left a comment

Choose a reason for hiding this comment

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

Thanks for updating this, I'll finish the review hopefully before... or after the KubeCon.

@idealhack
Copy link
Member

I'll take the time to read through docs and discussions about the change to merge in test-infra responsibilities too.

@Bubblemelon
Copy link
Contributor Author

Re:

can you squash these commits?

Yup, I'll squash all the commits into one when the changes are finalized 😄

@Bubblemelon
Copy link
Contributor Author

Note to self: Add shadow requirements/expectations

@palnabarun
Copy link
Member

palnabarun commented Jun 21, 2019

Hi everyone,

I see that the shadows are already decided for this role. Are there any more available shadow positions?

I was really interested to be working in the release team as a Branch Manager Shadow.

PS: I am a first timer. :)

@justaugustus
Copy link
Member

@palnabarun -- the application for Kubernetes 1.16 shadows is still open! Please fill it out here: https://docs.google.com/forms/d/e/1FAIpQLSe5voYfOk1pDyjBwotC0go8u0iFtiUpN8iN0LZ8K15dezW6oQ/viewform

@idealhack
Copy link
Member

idealhack commented Jun 21, 2019

@palnabarun hey, thanks for your interests in this role. we haven't decided shadows for this cycle yet, so feel free to apply for it and/or other roles :)

Copy link
Member

@tpepper tpepper left a comment

Choose a reason for hiding this comment

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

All in all very nice restructure and streamlining. I've made quite a few comments but all are relatively minor requests I think. The two main things I'm seeing:

  • move the TODO/NB's out of the document into issues (mea culpa...I should have done that multiple versions ago). That could also be left for a shadow to drive discussions and Q&A with them?
  • with the other good ordering/sectioning refactors, the need to shift the code management tasks (branchff, reverts, cherry picks) into a cohesive section of its own seems more apparent now.

Thanks for taking the time to do all this cleaning!

(also should squash the commits into one)

@Bubblemelon Bubblemelon force-pushed the brnchmgmt-handbook-update branch 4 times, most recently from aa562a0 to c23ff76 Compare June 24, 2019 22:23
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 25, 2019
@Bubblemelon Bubblemelon force-pushed the brnchmgmt-handbook-update branch from c23ff76 to c1f1644 Compare June 25, 2019 23:25
@k8s-ci-robot k8s-ci-robot added area/release-eng Issues or PRs related to the Release Engineering subproject and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jun 25, 2019
@Bubblemelon Bubblemelon force-pushed the brnchmgmt-handbook-update branch from c1f1644 to 14cc540 Compare June 25, 2019 23:29
Summary of changes:
- removed TODO items
- update content for each existing section
- rename sections, split and reposition
- add section on debugging and background information
- add configure merge automation and create presubmit jobs from test-infra role
- add test-infra handbook's useful links
- fix markdown format and rephrase
@Bubblemelon
Copy link
Contributor Author

This issue #696 was opened to group the TODO items from the handbook in one place.

@justaugustus
Copy link
Member

Thanks @Bubblemelon; looks great! :)
/lgtm
/approve

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Bubblemelon, justaugustus

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 26, 2019
@justaugustus
Copy link
Member

/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 Jun 26, 2019
@k8s-ci-robot k8s-ci-robot merged commit a746606 into kubernetes:master Jun 26, 2019
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. area/release-eng Issues or PRs related to the Release Engineering subproject area/release-team Issues or PRs related to the release-team subproject 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/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants