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

docs: Clarify coordination for backporting process #15989

Merged
merged 1 commit into from May 21, 2021
Merged

docs: Clarify coordination for backporting process #15989

merged 1 commit into from May 21, 2021

Conversation

christarazi
Copy link
Member

Document the common workflow that we've been working under.

Signed-off-by: Chris Tarazi chris@isovalent.com

@christarazi christarazi requested review from a team as code owners May 3, 2021 21:31
@christarazi christarazi added area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. sig/contributing Impacts contribution workflow, guidelines, and tools. labels May 3, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label May 3, 2021
@christarazi christarazi added the release-note/misc This PR makes changes that have no direct user impact. label May 3, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label May 3, 2021
@christarazi christarazi requested a review from glibsm May 3, 2021 21:31
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.10.0-rc2 May 3, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.9.7 May 3, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.8.10 May 3, 2021
qmonnet
qmonnet approved these changes May 4, 2021
View reviewed changes
Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

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

Non-blocking nits

Documentation/contributing/development/contributing_guide.rst Outdated Show resolved Hide resolved
Documentation/contributing/development/contributing_guide.rst Outdated Show resolved Hide resolved
Documentation/contributing/development/contributing_guide.rst Outdated Show resolved Hide resolved
@christarazi christarazi force-pushed the pr/christarazi/clarify-backporting-coordination branch from 739c22e to cdca453 Compare May 4, 2021 16:44
joestringer
joestringer reviewed May 5, 2021
View reviewed changes
Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

nit, who is "colleague" in this? How does someone figure that out? Do we mean "other hat owner for the week"?

@christarazi christarazi force-pushed the pr/christarazi/clarify-backporting-coordination branch from cdca453 to 1d51b0c Compare May 5, 2021 16:38
@christarazi
Copy link
Member Author

@joestringer Good point, updated

joestringer
joestringer approved these changes May 5, 2021
View reviewed changes
Copy link
Member

@joestringer joestringer left a comment •
edited

Choose a reason for hiding this comment

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

Looks good. I'd like to take it even maybe one step further, which is to say "Please coordinate with the previous / next hat to figure out how to get the backport PRs merged" so we don't get in this situation where there are 3 different PRs from 3 different weeks and none of them make progress because each hat owner works in a silo and gets stuck.

@christarazi christarazi force-pushed the pr/christarazi/clarify-backporting-coordination branch from 1d51b0c to 83aec56 Compare May 5, 2021 17:00
@christarazi
Copy link
Member Author

Another good point; I added a small paragraph describing as such.

joestringer
joestringer requested changes May 5, 2021
View reviewed changes
Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

Getting there, just a few more minor wording nits.

Documentation/contributing/development/contributing_guide.rst Outdated
Comment on lines 500 to 501
been merged, then please coordinate with the previous / next backporter to get
make progress on them.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
been merged, then please coordinate with the previous / next backporter to get
make progress on them.
been merged, then please coordinate with the previous / next backporter to check
what the status is and establish who will work on getting the backports into the tree
(for instance by investigating CI failures and addressing review feedback).

Copy link
Member

Choose a reason for hiding this comment

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

^ I realize it's a bit ambiguous here on "who" but from a high level the backports need to get done and release folks/maintainers don't care who does it as long as it gets done. I think it's useful to have some leeway for backporters of different weeks to negotiate who will work on the backports depending on who has the available time to do so.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've added a blurb mentioning the leeway and negotiation as well

Documentation/contributing/development/contributing_guide.rst Outdated
Comment on lines 492 to 493
The other hat owner will then end up doing v1.9 and v1.10 the next day, for
example.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The other hat owner will then end up doing v1.9 and v1.10 the next day, for
example.
The other hat owner can then handle v1.9 and v1.10 backports the next day, for
example.

@christarazi christarazi removed the request for review from glibsm May 20, 2021 17:15
@christarazi christarazi added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 20, 2021
@twpayne twpayne merged commit 946f52c into cilium:master May 21, 2021
@nbusseneau nbusseneau mentioned this pull request May 21, 2021
Merged
@nbusseneau
Copy link
Member

@christarazi I don't think we are maintaining 1.7 anymore, are we? Shall we remove the needs-backport/1.7 label?

@nbusseneau nbusseneau mentioned this pull request May 21, 2021
Merged
@nbusseneau
Copy link
Member

Confirmed here on public Slack, 1.7 is EOL: https://cilium.slack.com/archives/CDD1S4BHC/p1621620861010800?thread_ts=1621620829.010600&cid=CDD1S4BHC

@christarazi christarazi deleted the pr/christarazi/clarify-backporting-coordination branch May 21, 2021 18:51
@nbusseneau nbusseneau mentioned this pull request May 21, 2021
Merged
@aanm aanm moved this from Needs backport from master to Backport pending to v1.10 in 1.10.1 May 25, 2021
@aanm aanm moved this from Needs backport from master to Backport done to v1.9 in 1.9.8 May 27, 2021
@aanm aanm mentioned this pull request May 28, 2021
Merged
@aanm aanm moved this from Backport pending to v1.10 to Backport done to v1.10 in 1.10.1 Jun 16, 2021
@aanm aanm mentioned this pull request Jun 16, 2021
Merged
@joestringer joestringer moved this from Needs backport from master to Backport done to v1.8 in 1.8.11 Jun 30, 2021
@joestringer joestringer mentioned this pull request Jul 22, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@errordeveloper errordeveloper errordeveloper approved these changes

@joestringer joestringer joestringer approved these changes

@qmonnet qmonnet qmonnet approved these changes

Assignees
No one assigned
Labels
area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact. sig/contributing Impacts contribution workflow, guidelines, and tools.
Projects
No open projects
1.10.1
Backport done to v1.10
1.8.11
Backport done to v1.8
1.9.8
Backport done to v1.9
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@christarazi @nbusseneau @errordeveloper @joestringer @qmonnet @twpayne @kaworu @glibsm @aanm