Initial version of project rules #8359
Conversation
|
Start a new ephemeral environment with changes proposed in this pull request: |
openshift-ci
bot
added
the
do-not-merge/work-in-progress
There was a problem hiding this comment.
I think the biggest thing missing is the big picture. :-) Is this codifying the existing structure with minor changes/improvements? Or is this proposing a net-new contribution/release/... process?
It kinda seems like a step in the latter direction, but some key portions are missing (shared testing, org contribution, releases &c) -- which is why I think I'm confused. :-) And definitely seems like something we'd want input from the other groups on for a while, perhaps more formally as well (community calls &c).
Looks interesting though! More commentary inline. :)
RULES.md
Outdated
|
|
||
| Recommended way to get merge rights: Anybody who submits six non-trivial PRs that get merged in a period no longer than six months are encouraged to ask for rights. | ||
| A person can ask to be stripped of merge rights. | ||
| If a person doesn’t actively contribute to the project for a period of 12 months, their merge rights are stripped. Active contribution consists of either non-trivial contributions of code, non-trivial reviews, or analyses of problems in issues. Rights can be also revoked if coding style or conduct are consistently broken. |
There was a problem hiding this comment.
Just curious how this is going to work. ComplianceAsCode is admittedly a different org than OpenSCAP but we still have 15+ repos under the CaC org. Are we going to scope contributors to particular projects? Do merge bits apply across projects? If OpenSACAP/scap-workbench were in this org, it might be hard to get 6 non-trivial PRs in 6 months to that repo (I tried while working at Canonical and only managed 4 with 2 failed -- and some of those 4 were trivial! :D) and another 6 to CaC/content to have commit bits in both, but I could definitely see why the separation would be good. Would there be some threshold of contributions to be a "organization" member versus a "CaC/content contributor if that makes sense? Would the former retain merge commits longer than the latter? &c.
Also, is this strictly removing the merge bits or does it remove ACK'ing rights as well? E.g., assuming I get busy at HCP (likely) and don't have time here, it makes sense that I'd lose merge access, but would my review still count for +1 on PRs (allowing others to merge)? Or would it be +0?
Just asking the hard questions ;-)
There was a problem hiding this comment.
This is a guideline only for the this (ComplianceAsCode/content) project. The process of getting commit rights for other projects are not defined - whoever wants them can ask on MLs.
As noted below, 6 PRs in 6 months is the "standard" way, but not the only one. One can say that it encourages people to create smaller PRs, or to reach out if they don't start from zero.
ACKing is not defined, and I would say that any PR review helps - if the change doesn't have to be reviewed by SMEs (e.g. it is a fix of Bash content or of OVAL) such reviews help to get PRs through faster. I think that we can work with informal "ACK power" - if one's username sounds familiar, then they are likely to have the ACK power.
Concerning organizations, it is ATM a very loose concept. We will iron it out based on feedback and needs of contributing organizations.
There was a problem hiding this comment.
There's two types of ACKs in GitHub: ACKs by "approved" members (example) and ACKs by random community members (example). Even though I don't have merge bits in either repository, I've been added to a list of PR reviewers on the former repo, so my review still "counts" as a +1 rather than a +0. This lets someone who does have merge bits merge the PR without having to ACK it themselves (counts as an independent review in the case where PRs need to be ACKd before being merged).
I bring this up because these finer permissions might be useful to the project. SUSE &c could have many mergeless reviewers (and this could be given fairly liberally), but fewer people with merge access.
Perhaps rather than removing the user from the repo entirely (after 6 months or whatever), they could be dropped to approvals only for the next period of time?
My 2c.
There was a problem hiding this comment.
Edit to add (and in case anyone is wondering how this is accomplished), apparently I had "write" access to the repository at the time of this discussion -- but due to branch protections, I lacked permission to merge (since I couldn't write to the protected branch).
There was a problem hiding this comment.
I think that we don't have to distinguish between authoritative approvals and merges - I wouldn't look at it as stripping the approval when the merge right expires, but rather as keeping the merge right as long as we think that there is a solid chance of getting a qualified review out of that person.
As the period of long inactivity passes, I would suggest to treat people as "random community members" again, and it should be easier to regain merge rights, e.g. by contributing reviews.
RULES.md
Outdated
|
|
||
| ### Products | ||
|
|
||
| - Keeping track of the decomposition: Metadata in product.yml files, CODEOWNERS file |
There was a problem hiding this comment.
CODEOWNERS would be cool :-) Looks like we don't have any right now, so we'll probably have to ask for them retroactively in products? Might be cool to have Red Hat propose some for the existing content in parallel with this PR so we can see roughly what that'd look like both for RH properties and for CaC-the-build-system &c?
There was a problem hiding this comment.
Yes, we have to figure out how it works. You are right that initial version of CODEOWNERS should be a part of the PR when it is not a draft any more.
RULES.md
Outdated
| We need to: | ||
|
|
||
| Allow independent groups in the content project to work at their own pace on parts of the project that don’t influence each other. | ||
| Introduce enter and exit criteria for roles with elevated privileges so the hierarchy can self-organize. |
There was a problem hiding this comment.
nit: roles? Maybe say individuals and organizations? IMO there's both individual specific and org-specific stuff below, so perhaps roles might mean something like strictly an organization-nominated maintainers (RH, Canonical, SUSE, Oracle...) and exclude individual contributors unnecessarily... My 2c.
There was a problem hiding this comment.
Agreed with @cipherboy, it's generally clearer to use specifics, whether that's individuals and organizations or what have you, as opposed to roles. Rules is too flexible of a term. :)
RULES.md
Outdated
| ## TLDR | ||
| We need to: | ||
|
|
||
| Allow independent groups in the content project to work at their own pace on parts of the project that don’t influence each other. |
There was a problem hiding this comment.
The one thing I don't know if I've seen discussed is how releases will be handled. If other groups are doing upstream-released based distribution (versus point-in-time-tagging), it might be nice to have coordination around release time to ensure the bits are stable across all products? That might just be a designated/published cadence for releases and criteria for backports (which RH mostly maintains now), or it could be voting on releases &c. I do not know :-)
A distro like Fedora builds all content, and perhaps others (like Debian or OpenSUSE?) might as well in the future, so perhaps there's value in considering this.
There was a problem hiding this comment.
The release schedule is time-based (every two months) with occasional <2w delays. Of course it would be nice to have other parties more aware and consciously participating. Release times are public - just check out associated milestones.
We don't have a concept of backports - everybody is supposed to take care of this downstream, and we have observed that the increasing quality of the project made this less of a problem. There is also a public stabilization period to get bugfixes through.
RULES.md
Outdated
| In order to keep the project scalable, we divide the project into separate areas, and assign groups of users that can be granted commit access with the intention to develop content in respective areas. | ||
| Those areas are going to be segmented by | ||
|
|
||
| - Products (RHEL, Ubuntu, …) - make it easy to develop content of a product that doesn’t influence the rest of the build system. |
There was a problem hiding this comment.
How's this managed hierarchically? Wouldn't Ubuntu contributors necessarily have access to all Ubuntu-product-specific profiles? Or is this additive rather than absolute (e.g., any of the Ubuntu contributors/maintainers plus the CIS-specific ones in the example below)?
RULES.md
Outdated
|
|
||
| - Products (RHEL, Ubuntu, …) - make it easy to develop content of a product that doesn’t influence the rest of the build system. | ||
| - Product-specific profiles (PSPs s.a. Firefox STIG, Ubuntu CIS, …) and respective control files - make it easy for SMEs that only want to assign rules to profiles without going into details. | ||
| - Product-independent profiles (PIPs s.a. ANSSI, HIPAA, PCI-DSS, …) and respective control files - although PIP development that benefits the whole community instead of cluttering the content with “if product in [...]”. |
There was a problem hiding this comment.
I forget, controls aren't presently compiled and checked-in, right? So these product-independent profiles, would they need the review of any PSP authors if they overlap (e.g., CIS control)?
There was a problem hiding this comment.
There are controls that are coupled with PSPs s.a. the CIS ones, while ANSSI controls are used by the PIP ANSSI family. Does this answer your question?
RULES.md
Outdated
| ### Products | ||
|
|
||
| - Keeping track of the decomposition: Metadata in product.yml files, CODEOWNERS file | ||
| - Area of effect: All PSPs that don’t have specific stakeholders, associated content and test scenarios. |
There was a problem hiding this comment.
It looks like my question above about additive vs. net is described here, but I'm not quite sure if that can be reflected in a CODEOWNERS file well? Maybe I'm wrong, its been a while since I looked at that, so perhaps the suggestion of a RH example would be good here.
Are test scenarios something handled automatically (e.g., ssgts and are we going to be running that across products on releases)? Or documented in the project? Or just internally at specific orgs?
There was a problem hiding this comment.
I don't understand what exactly you mean by additive vs net or what is the "handling" of test scenarios.
RULES.md
Outdated
| The project maintainers decide about granting or strip of rights and about exceptions to the procedure e.g. when an applicant has deep prior experience with the project. | ||
|
|
||
|
|
||
| ### Organizations |
There was a problem hiding this comment.
I'm not sure I really see how the below paragraph meshes with the above. Will e.g., SUSE have only a single merge-bits maintainer (+ 1 backup)? Or if they had multiple SUSE-employed contributors above the threshold, they'll get all of them under the above 6 month term limit plus one (+ 1 backup) "long term" 12-month merge bits people?
There was a problem hiding this comment.
The idea is that an organization will get one more person with merge access "for free". This and the project administration are actually the only places where the "organization" concept is effective.
|
@Xeicker @dodys @anivan-suse any input on this? I tagged you based on your latest contributions for different distros. |
The goal of this PR is to move into the direction of the option 2 in a slow pace. This proposal shouldn't restrict anybody, except it can serve as a basis of removing merge rights from long-gone contributors. For everybody else, it should encourage and extend contribution possibilities. We are trying to get as much feedback as possible now, and if there is an interest for community calls, we will gladly host those. If we merge this guideline and it turns out that it has holes or that it presents obstacles to contributors, we will change it - it won't be carved in stone, and its purpose is again to encourage and to share responsibility, not to introduce some sort of benevolent dictatorship. |
RULES.md
Outdated
| Allow independent groups in the content project to work at their own pace on parts of the project that don’t influence each other. | ||
| Introduce enter and exit criteria for roles with elevated privileges so the hierarchy can self-organize. | ||
| Define a framework for organizations to join the project and assume responsibilities. | ||
| Make sure that the project stays coherent and individual parts contribute to its value, and the project doesn’t become a disgusting spaghetti soup. |
There was a problem hiding this comment.
nit: Ensure instead of Make sure
There was a problem hiding this comment.
Although, right after commenting that I remembered this is a draft so you can ignore that :)
|
I agree with @cipherboy about some missing items like shared testing, org contribution, etc., although since this is still in the draft phase perhaps that's something that will come later. |
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
|
I have addressed feedback that was raised, and moved the text to the developer guide, so it shows up in the Introduction section on the readthedocs site. Notable feedback omissions:
What will happen when this PR gets merged? Nothing notable. I hope that we will see applications for merge rights, and we can polish the project decomposition e.g. in #8482. Remember that the purpose of those governance rules is to open the project up without causing a disaster. |
There was a problem hiding this comment.
I liked this initial version of project rules. The feedback comments were fairly addressed and the current status is simple, clear and objective. We can also continually improve it once we put the rules in practice. Thanks @matejak and all contributors.
Create an initial version of "traffic rules" - a document that would help the project community to self-organize and deliver required changes at a faster pace, and ensure that various parties can collaborate on the project without major friction.
Please disregard that the file changed is a weird
RULES.mdthat nobody ever heard of - the main issue is to agree on the text of the guideline, and then it will most likely be distributed across existing files.