-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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
doc: changes to merge criteria (4 eyes principle) #62122
Conversation
doc/project/project_roles.rst
Outdated
* Release engineers shall not merge code changes originating from the same | ||
organisation if all other reviewers are also from the same organisation. | ||
To be able to merge such changes, at least one review shall be from a | ||
different organisation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about ?
A change coming from one organisation needs at least an approval from another organisation to be merged
doc/project/project_roles.rst
Outdated
submitters' organisation). | ||
* Changes or additions to hardware support (driver, SoC, boards) shall at | ||
least have the merger be from a different organisation. | ||
* Release engineers shall not merge code changes originating from the same |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not apply for certain types of changes in my opinion. There are just too many changes that are very difficult to review for someone who is not familiar with the hardware or area. I would again differentiate between introducing new things and maintaining them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To me, it is looking much better with this phrasing.
A few more minor things.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just very minors at this point from me.
Reference to slide that does not exist. Signed-off-by: Anas Nashif <anas.nashif@intel.com>
Document 4 eye principal for reviews and merges. Signed-off-by: Anas Nashif <anas.nashif@intel.com>
each affected area; Unless the changes to a given area are considered trivial | ||
enough, in which case approvals by other affected subsystems | ||
maintainers/collaborators would suffice. | ||
* Four eye principle on the organisation level. We already require at least 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* Four eye principle on the organisation level. We already require at least 2 | |
* Four eyes principle on the organisation level. We already require at least 2 | |
approvals (basic four eyeS principle), however, such reviews and approvals |
one review shall be from a different organisation. | ||
|
||
* A minimum review period of 2 business days, 4 hours for trivial changes (see | ||
:ref:`review_time`). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find these very short but off-topic...
Fixes #61358