Skip to content

Document change review process - #132

Merged
mnm678 merged 4 commits into
theupdateframework:masterfrom
joshuagl:joshuagl/change-review
Nov 25, 2020
Merged

Document change review process#132
mnm678 merged 4 commits into
theupdateframework:masterfrom
joshuagl:joshuagl/change-review

Conversation

@joshuagl

@joshuagljoshuagl commented Oct 30, 2020

Copy link
Copy Markdown
Member

In-line with what is discussed in #130 here's a first stab at documenting our change review process. I don't know whether 5 business days is long enough a contemplation period, or whether the term "business days" is clear enough. I'd very much welcome feedback on those aspects, as well as the rest of the proposal.

In order to ensure it's clear who will be doing the review, I've also added a MAINTAINERS.md listing the consensus builder and active TAP editors. Hopefully we can link to this document from TAP 1 so that it's clear to participants in the TAP process who the TAP Editors are.

Significant inspiration was taken from SPIFFE's GOVERNANCE.md

@joshuagljoshuagl mentioned this pull request Oct 30, 2020
Closed
joshuagl
joshuagl commented Nov 2, 2020
View reviewed changes
Comment threadGOVERNANCE.md Outdated
mnm678
mnm678 previously approved these changes Nov 2, 2020
View reviewed changes

@mnm678mnm678 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

A couple of minor comments, but this looks great.

Comment threadGOVERNANCE.md Outdated
trishankatdatadog
trishankatdatadog reviewed Nov 4, 2020
View reviewed changes
Comment threadMAINTAINERS.md
Comment threadGOVERNANCE.md Outdated
@joshuagl

joshuagl commented Nov 24, 2020

Copy link
Copy Markdown
MemberAuthor

I think we're all agreed on this, but in the spirit of the proposed changes I'd appreciate a second approval.

cc @JustinCappos@trishankatdatadog@lukpueh

This was referenced Nov 24, 2020
Merged
Merged
Merged
trishankatdatadog
trishankatdatadog previously approved these changes Nov 24, 2020
View reviewed changes
@trishankatdatadog

trishankatdatadog commented Nov 24, 2020

Copy link
Copy Markdown
Contributor

We should change the branch protection rules to require >= 2 reviews...

@lukpueh

lukpueh commented Nov 25, 2020

Copy link
Copy Markdown
Member

We should change the branch protection rules to require >= 2 reviews...

Done.

@joshuagl
joshuaglforce-pushed the joshuagl/change-review branch from 07b2b9e to f271b48CompareNovember 25, 2020 10:26
@joshuagl
Add a MAINTAINERS file
54aa2eb
Document the consensus builder and all active TAP Editors.
Signed-off-by: Joshua Lock <jlock@vmware.com>
@joshuagl
Add GOVERNANCE file documenting change review
9f2f93b
Define the roles of participants in the project and document the expected
change review process for the specification.
Signed-off-by: Joshua Lock <jlock@vmware.com>
@joshuagl
State contemplation period in weeks, not business days
fa5b8d1
What constitutes a business day may vary by region.
Weeks are more universal.
Signed-off-by: Joshua Lock <jlock@vmware.com>
@joshuagl
Clarify 'in a state which can be merged'
3799328
Signed-off-by: Joshua Lock <jlock@vmware.com>
@joshuagl
joshuaglforce-pushed the joshuagl/change-review branch from f271b48 to 3799328CompareNovember 25, 2020 10:26
@joshuagl

joshuagl commented Nov 25, 2020

Copy link
Copy Markdown
MemberAuthor

Apologies, I forced push to squash in some of the fixup commits introduced during code review and that has dismissed the existing reviews.

Will need re-approval to meet the branch protection rules. Thank you in advance.

@mnm678

mnm678 commented Nov 25, 2020

Copy link
Copy Markdown
Collaborator

hmm, the Travis version checks are failing. This doesn't change the specification itself, so I'm going to go ahead and merge.

@mnm678
mnm678 merged commit 9a41a47 into theupdateframework:masterNov 25, 2020
@joshuagl
joshuagl deleted the joshuagl/change-review branch November 26, 2020 10:03
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mnm678mnm678mnm678 approved these changes
+1 more reviewer
@trishankatdatadogtrishankatdatadogtrishankatdatadog approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@joshuagl@trishankatdatadog@lukpueh@mnm678