Conversation
pastaq
left a comment
There was a problem hiding this comment.
Some minor nits and clarification requests
|
|
||
| ## 1. Introduction | ||
|
|
||
| The Open Gaming Collective (OGC) maintains a custom Linux kernel intended for use by downstream distributions and their users. This proposal describes the recommended repository layout, workflows, and policies for developing, maintaining, and distributing OGC kernel patches. |
There was a problem hiding this comment.
Let's adjust the verbage. Instead of proposal let's refine it as the accepted governance document in some way.
There was a problem hiding this comment.
Yeah I'll update that :)
|
|
||
| * Patches maintained by third parties (e.g. CachyOS, Linux stable). | ||
| * The OGC org will merge and keep up with these patches. | ||
| * These patches are also periodically reviewed for relevance and maintenance cost. |
There was a problem hiding this comment.
Can we add an example? This seems nebulous and I can't imagine a scenario where these don't match one of the two other options.
There was a problem hiding this comment.
I don't know exactly what you mean. But I can imagine a case where we would drop a certain patchset due to high maintenance or not able to rebase or whatever.
There was a problem hiding this comment.
In what scenario would adding a patch fall under this category?
|
|
||
| ### 4.5 Pull Request Requirements | ||
|
|
||
| * All supported package formats are built for every pull request. |
There was a problem hiding this comment.
I feel like this should be automatic for members with the workflow triggered manually by the member reviewer from outside collaborators
We should also have a separate workflow for validation (no warnings, checkpatch --strict passing, documentation validation etc )
There was a problem hiding this comment.
|
|
||
| * Only merge commits and tag commits are permitted. | ||
| * Updates to patch series and/or linux-stable are applied via merges only. | ||
| * This means that these branches will not be rebased for a clear history. |
There was a problem hiding this comment.
We can always choose to change this in the future if it is not workable. But for me, a clean history is more important here.
|
|
||
| ### 5.4 Release and Distribution Model | ||
|
|
||
| * **Rolling distributions:** |
There was a problem hiding this comment.
Do we have any rolling release members?
There was a problem hiding this comment.
I think Bazzite is this under a technicality only. We're based on Fedora versions but users are updated to the latest version automatically. Whether that matters enough to be a distinction here is another question.
There was a problem hiding this comment.
Yeah I just wanted to be flexible for downstreams here. I think we should default to packages, but if a container format is wanted I don't want to block that.
| * The history of `/OGC` branches **must not be rewritten**. | ||
|
|
||
| * Only merge commits and tag commits are permitted. | ||
| * Updates to patch series and/or linux-stable are applied via merges only. |
There was a problem hiding this comment.
I think in this model it's especially important that you have metadata in the commit message to indicate where code came from or it's going to be incredibly difficult to unwind.
For example cherry picks must be run with -x and patches from mailing lists need to have the URL added to commit messages.
There was a problem hiding this comment.
Adding to that, I think all commits should be signed by the person requesting the merge, so -s should also be required.
There was a problem hiding this comment.
It might be good to have an entire section dedicated to workflow detailing on how to pull in changes from a URL, another branch, a patch file, etc.
There was a problem hiding this comment.
I agree, but is it OK to do that in a seperate PR?
|
Approved based on existing conversations and their resolutions, just adding my +1 |
|
I added an additional document to the branch detailing the procedure for adding patches to the OGC kernel. |
7336eb4 to
44cb3b8
Compare
|
Can we merge this? Seems the only change request we've accepted fixing in a separate pr |
44cb3b8 to
14955fb
Compare
After our discussion on Discord I wanted to place this here.