Patch Triage - Revision history

RossBurton at 10:19, 13 October 2023

2023-10-13T10:19:38Z

@@ Line 22: / Line 22: @@
 === Patch State Summary ===
-[[File:patchworkflow.png]]
+{{#mermaid:flowchart TD
 * New: Nobody has reviewed it yet

RossBurton at 12:57, 12 October 2023

2023-10-12T12:57:28Z

← Older revision		Revision as of 12:57, 12 October 2023
Line 21:		Line 21:

	=== Patch State Summary ===		=== Patch State Summary ===

			[[File:patchworkflow.png]]

	* New: Nobody has reviewed it yet		* New: Nobody has reviewed it yet

RossBurton at 17:40, 11 October 2023

2023-10-11T17:40:58Z

← Older revision		Revision as of 17:40, 11 October 2023
Line 33:		Line 33:
	'''TODO''' There is ambiguity around the "Accepted" state. Should this indicate a patch which ''is ready to be merged'', or only for patches that ''have already been merged''? Glibc has a "committed" state to handle this.		'''TODO''' There is ambiguity around the "Accepted" state. Should this indicate a patch which ''is ready to be merged'', or only for patches that ''have already been merged''? Glibc has a "committed" state to handle this.

			'''TODO''' Now thinking Under Review until merged (accepted) or rejected (rejected).

	=== Minutes ===		=== Minutes ===

RossBurton at 17:26, 11 October 2023

2023-10-11T17:26:35Z

← Older revision		Revision as of 17:26, 11 October 2023
Line 1:		Line 1:
	~~I propose that we add~~ a ~~new~~ meeting to ~~https://www.yoctoproject.org/public-virtual-meetings/~~, ~~similar to the Bug Triage meeting but to~~ review incoming patches. The patch queue is long and whilst we talk about how patches-on-list means anyone can perform drive-by review, there’s no way to know if someone has actually reviewed a patch and mentally said “this looks good”, or if several people have seen a patch and been sceptical but not actually posted their thoughts. This proposal aims to counter that problem.		The Yocto Project community is experimenting with a regular patchwork triage meeting; to triage, review, and assign incoming patches.

	~~We have a patchwork instance at patchwork.yoctoproject.org which has the ability~~ to ~~track patches~~, ~~their state, and assign them to a specific person. This~~ is the ~~bare minimum of a patch triage process, we just need to use it~~. I admit that what I’m proposing here is inspired by the glibc project process (sourceware.org/glibc/wiki/PatchworkReviewMeetings), because I learnt about their process whilst attending the GNU Cauldron conference.		Everyone is welcome to attend, the call is held on Thursdays at 08:30 - 09:30 PST (the hour after the Bug Triage call). Other meetings may be added to cater for other timezones or if there are more patches than can be triaged.

	~~As we have a fairly constant stream~~ of patches~~, I expect these triage meetings should be done twice weekly, to allow for rapid feedback and review cycles. Each meeting would be an hour long~~: ~~the first half spent quickly reviewing as much of the patch queue as possible with any discussion postponed until the second half~~. ~~This means the attendees can hopefully work through a large number of patches quickly before circling back on the more interesting patches~~. ~~Any patches which result in a bigger discussion or have open questions should likely be discusseed in the Engineering Sync, where the attendence will be larger. As patchwork is primarily a read~~-~~only system (apart from status and delegation), a wiki page will be needed to keep minutes and notes.~~		The focus of the meeting is to triage new patches in patchwork: https://patchwork.yoctoproject.org/project/oe-core/list/
	~~I propose the process to be as follows:~~

	~~1. Obtain last patch ID reviewed from the meeting minutes~~		=== Process ===

	~~2. Iterate through the patches since the last patch reviewed, oldest first, and review. Quickly review as a group and assign a delegate and provisional status:~~		This process is inspired by the process the [https://sourceware.org/glibc/wiki/PatchworkReviewMeetings glibc project] use.
	~~– Looks Good: move to Under Review status. Will be incorporated into a staging branch~~ by the ~~delegate so that if it fails testing the status can be changed.~~
	~~– Looks Bad: move to Rejected status. Note the feedback in the minutes, the delegate is responsible for responding on~~ the ~~list with this feedback.~~
	~~– Needs Work (quick)~~: ~~move to Action Required. Note the feedback in the minutes, the delegate is responsible for responding on the list with this feedback.~~
	~~– Needs Further Review. Move to Action Required. Note this choice in the minutes, and either discuss later in the call or in the Engineering Sync~~. ~~It is the responsiblity of the delegate to have this discussion~~.

	3. Discuss any patches marked a Needs Further Review, and any other patches that the group wish to discuss.		# Obtain last patch ID reviewed from the minutes
			# Iterate through the patches since the last patch reviewed, oldest first. Quickly review as a group and assign a delegate and provisional status:
			** Looks Good: move to Under Review status. Will be incorporated into a staging branch by the delegate so that if it fails testing the status can be changed.
			** Looks Bad: move to Rejected status. Note the feedback in the minutes, the delegate is responsible for responding on the list with this feedback.
			** Needs Work (quick): move to Action Required. Note the feedback in the minutes, the delegate is responsible for responding on the list with this feedback.
			** Needs Further Review. Move to Action Required. Note this choice in the minutes, and either discuss later in the call or in the Engineering Sync. It is the responsiblity of the delegate to have this discussion.
			# Discuss any patches marked a Needs Further Review, and any other patches that the group wish to discuss.
			# Review the state of all patches that have been delegated but not yet resolved.
			# Note down in the minutes the last patch ID reviewed, for the next meeting.

	~~4. Review the state of all patches that have been delegated but not yet resolved.~~

	~~5. Note down in the minutes the last patch ID reviewed, for the next meeting.~~		=== Patch State Summary ===

	As a ~~straw~~-~~man proposal, I suggest Monday and Thursday~~ for the ~~calls~~. ~~The hour slot immedatiately after bug triage~~?		* New: Nobody has reviewed it yet
			* Under Review: The patch is under review and testing. Change the delegate to the reviewer.
			* Accepted: There is consensus for merging this patch.
			* Rejected: There is consensus for not merging this patch.
			* RFC: The patch is a Request for Comments to solicit feedback and doesn't need to be tracked further.
			* Change Requested: The patch has been reviewed and needs further work.
			* Superseded - A newer version of the patch exists.
			* Deferred - Review of this patch is waiting for another event to happen.

			'''TODO''' There is ambiguity around the "Accepted" state. Should this indicate a patch which ''is ready to be merged'', or only for patches that ''have already been merged''? Glibc has a "committed" state to handle this.


			=== Minutes ===

			==== 2023-10-12 ====

			...

RossBurton: Created page with "I propose that we add a new meeting to https://www.yoctoproject.org/public-virtual-meetings/, similar to the Bug Triage meeting but to review incoming patches. The patch queue..."

2023-10-11T12:54:18Z

Created page with "I propose that we add a new meeting to https://www.yoctoproject.org/public-virtual-meetings/, similar to the Bug Triage meeting but to review incoming patches. The patch queue..."

New page

I propose that we add a new meeting to https://www.yoctoproject.org/public-virtual-meetings/, similar to the Bug Triage meeting but to review incoming patches. The patch queue is long and whilst we talk about how patches-on-list means anyone can perform drive-by review, there’s no way to know if someone has actually reviewed a patch and mentally said “this looks good”, or if several people have seen a patch and been sceptical but not actually posted their thoughts. This proposal aims to counter that problem.

We have a patchwork instance at patchwork.yoctoproject.org which has the ability to track patches, their state, and assign them to a specific person. This is the bare minimum of a patch triage process, we just need to use it. I admit that what I’m proposing here is inspired by the glibc project process (sourceware.org/glibc/wiki/PatchworkReviewMeetings), because I learnt about their process whilst attending the GNU Cauldron conference.

As we have a fairly constant stream of patches, I expect these triage meetings should be done twice weekly, to allow for rapid feedback and review cycles. Each meeting would be an hour long: the first half spent quickly reviewing as much of the patch queue as possible with any discussion postponed until the second half. This means the attendees can hopefully work through a large number of patches quickly before circling back on the more interesting patches. Any patches which result in a bigger discussion or have open questions should likely be discusseed in the Engineering Sync, where the attendence will be larger. As patchwork is primarily a read-only system (apart from status and delegation), a wiki page will be needed to keep minutes and notes.
I propose the process to be as follows:

1. Obtain last patch ID reviewed from the meeting minutes

2. Iterate through the patches since the last patch reviewed, oldest first, and review. Quickly review as a group and assign a delegate and provisional status:
– Looks Good: move to Under Review status. Will be incorporated into a staging branch by the delegate so that if it fails testing the status can be changed.
– Looks Bad: move to Rejected status. Note the feedback in the minutes, the delegate is responsible for responding on the list with this feedback.
– Needs Work (quick): move to Action Required. Note the feedback in the minutes, the delegate is responsible for responding on the list with this feedback.
– Needs Further Review. Move to Action Required. Note this choice in the minutes, and either discuss later in the call or in the Engineering Sync. It is the responsiblity of the delegate to have this discussion.

3. Discuss any patches marked a Needs Further Review, and any other patches that the group wish to discuss.

4. Review the state of all patches that have been delegated but not yet resolved.

5. Note down in the minutes the last patch ID reviewed, for the next meeting.

As a straw-man proposal, I suggest Monday and Thursday for the calls. The hour slot immedatiately after bug triage?