From c15eb54fa10b504435d608880a82145d1a336764 Mon Sep 17 00:00:00 2001 From: joeyaiello Date: Wed, 20 Jul 2016 10:42:08 -0700 Subject: [PATCH 1/6] first pass of governance doc still need to do a considerable rewrite of other contributor docs in order to change terminology to align with governance.md --- docs/community/governance.md | 182 +++++++++++++++++++++-- docs/dev-process/pull-request-process.md | 47 ++++++ docs/maintainers/README.md | 4 +- 3 files changed, 220 insertions(+), 13 deletions(-) create mode 100644 docs/dev-process/pull-request-process.md diff --git a/docs/community/governance.md b/docs/community/governance.md index 3a2941b15..e89e9de9b 100644 --- a/docs/community/governance.md +++ b/docs/community/governance.md @@ -1,12 +1,172 @@ -author: Joey -> PowerShell community, twitter, facebook, linkedIn...) -> Community ground rules, appeals process (if they disagree with our decision) -> SME +# PowerShell Governance -TODO -1. Define a difference between small, medium, large design decisions/bugs - a. Point to quick fix doc -2. Explain the RFC process -3. Define maintainer vs. feature owner -4. Define committee -5. \ No newline at end of file +## Terms + +* [**PowerShell Committee**](#powershell-committee): A committee of project owners who are responsible for design decisions, approving [RFCs][RFC-repo], and approving new maintainers/committee members +* [**Repository maintainer**](#repository-maintainer): An individual responsible for merging pull requests (PRs) into `master` when all requirements are met (code review, tests, docs, and RFC approval as applicable). +Repository Maintainers are the only people with write permissions into `master`. +* [**Area experts**](#area-experts): People who are experts for specific components (e.g. PSReadline, the parser) or technologies (e.g. security, performance). +Area experts are responsible for code reviews, issue triage, and providing their expertise to others. +* **Corporate Maintainer**: The Corporate Maintainer is an entity, person or set of persons, with the ability to veto decisions made by the PowerShell Committee or any other collaborators on the PowerShell project. +This veto power will be used with restraint since it is intended that the community drive the project. +Under extreme circumstances the Corporate Maintainer also reserves the right to dissolve or reform the PowerShell Committee. +The Corporate Maintainer for PowerShell is Microsoft. +* [**RFC process**][RFC-repo]: The "review-for-comment" (RFC) process whereby design decisions get made. + +## PowerShell Committee + +The PowerShell Committee and its members (aka Committee Members) are the primary (TODO: stewards, powerShell experience) decision makers of the PowerShell language, design, and project. + +### Committee Member Responsibilities + +Committee Members are responsible for reviewing and approving [PowerShell RFCs][RFC-repo] proposing new features or design changes. + +#### Changes that require an [RFC][RFC-repo] + +The following types of decisions require a written RFC and ample time for the community to respond with their feedback before a contributor begins work on the issue: + +* new features or capabilities in PowerShell (e.g. PowerShell classes, PSRP over SSH, etc.) +* anything that might require a breaking changes as defined in our [Breaking Changes Contract][breaking-changes] +* new modules, cmdlets, or parameters that ship in the core PowerShell modules (e.g. `Microsoft.PowerShell.*`, `PackageManagement`, `PSReadline`) +* the addition of new PowerShell Committee Members or Repository Maintainers +* any changes to the process of maintaining the PowerShell repository (including the responsibilities of Committee Members, Repository Maintainers, and Area Experts) + +#### Changes that don't require an RFC + +In some cases, a new feature or behavior may be deemed small enough to forgo the RFC process +(e.g. changing the default PSReadline `EditMode` to `Emacs` on Mac/Linux). +In these cases, [issues marked as `1 - Planning`][issue-process] require only a simple majority of Committee Members to sign off. +After that, a Repository Maintainer should relabel the issue as `2 - Ready` so that a contributor can begin working on it. + +If any Committee Members feels like this behavior is large enough to warrant an RFC, they can add the label `RFC-required` and the issue owner is expected to follow the RFC process. + +#### Committee Member DOs and DON'Ts + +As a PowerShell Committee Member: + +1. **DO** assign issues to area experts or repository maintainers as appropriate +1. **DO** reply to issues and pull requests with design opinions +(this could include offering support for good work or exciting new features) +1. **DO** encourage healthy discussion about the direction of PowerShell +1. **DO** raise "red flags" on PRs that haven't followed the proper RFC process when applicable +1. **DO** contribute to documentation and best practices +1. **DO** maintain a presence in the PowerShell community outside of GitHub (Twitter, blogs, StackOverflow, Reddit, Hacker News, etc.) +1. **DO** heavily incorporate community feedback into the weight of your decisions +1. **DO** be polite and respectful to a wide variety of opinions and perspectives +1. **DO** make sure contributors are following the [contributor guidelines](../../.github/CONTRIBUTING.md) + +1. **DON'T** constantly raise "red flags" for unimportant or minor problems to the point that the progress of the project is being slowed +1. **DON'T** offer up your opinions as the absolute opinion of the PowerShell Committee. +Members are encouraged to share their opinions, but they should be presented as such. + +### PowerShell Committee Membership + +The initial PowerShell Committee consists of Microsoft employees. +It is expected that over time, PowerShell experts in the community will be made Committee Members. +Membership is heavily dependent on the level of contribution and expertise: individuals who contribute in meaningful ways to the project will be recognized accordingly. + +At any point in time, a Committee Member can nominate a strong community member to join the Committee. +Nominations should be submitted in the form of [RFCs][RFC-repo] detailing why that individual is qualified and how they will contribute. +After the RFC has been discussed, a unanimous vote will be required for the new Committee Member to be confirmed. + +### Current Committee Members + +* Bruce Payette +* Jason Shirk +* Steve Lee +* Hemant Mahawar +* Joey Aiello ([joeyaiello](https://github.com/joeyaiello)) + +## Repository Maintainers + +Repository Maintainers are trusted stewards of the PowerShell repository responsible for maintaining consistency and quality of PowerShell code. +One of their primary responsibilities is merging pull requests after all requirements have been fulfilled. + +Repository Maintainers have [write access](https://help.github.com/articles/permission-levels-for-an-organization-repository/) to the PowerShell repository which gives them the power to: + +1. Merge pull requests to all branches *including* `master`. +1. `git push` to all branches *including* `master`. +1. Correctly assigning labels, milestones, and contributors to [issues](https://guides.github.com/features/issues/) + +### Repository Maintainer Responsibilities + +Repository Maintainers enable rapid contributions while maintaining a high level of quality in PowerShell by ensuring that all development processes are being followed correctly. + +If you are a Repository Maintainer: + +1. **DO** add [the correct labels](../dev-process/issue-label-descriptions.md) to issues and pull requests +1. **DO** make sure that [any change requiring approval from the PowerShell Committee](#committee-member-responsibilities) has gone through the proper [RFC][RFC-repo] or approval process +1. **DO** make sure the correct [Area Experts](#area-experts) are assigned to relevant pull requests and issues. +This includes adding extra reviewers when it makes sense +(e.g. a pull request that adds remoting capabilities might require a security expert) +1. **DO** validate that code reviews have been performed before merging a pull request +1. **DO** validate that applicable tests and documentation have been written before merging a pull request +1. **DO** make sure contributors are following the [contributor guidelines](../../.github/CONTRIBUTING.md) +1. **DO** ask people to resend a pull request, if it [doesn't target `master`](../../.github/CONTRIBUTING.md#lifecycle-of-a-pull-request). +1. **DO** wait for the [CI system][ci-system] build to pass for pull requests. +1. **DO** encourage contributors to refer to issues in their pull request description per the [issue template](../../.github/ISSUE_TEMPLATE) (e.g. `Resolves issue #123`) +1. **DO** encourage contributors to create meaningful titles for all PRs. +Edit the title if necessary to provide clarity on the problem +1. **DO** verify that all contributors are following the [Coding Guidlines](../dev-process/coding-guidelines.md) +1. **DO** ensure that each contributor has signed a valid Contributor License Agreement (CLA) +1. **DO** verify compliance with any third party code license terms (e.g., requiring attribution, etc.) if the contribution contains third party code + +1. **DON'T** merge pull requests with a failed CI build into `master` +1. **DON'T** merge pull requests without the label `cla-signed` or `cla-not-required` from the Microsoft CLA bot +1. **DON'T** merge pull requests that do not [include all meaningful changes](../../.github/CONTRIBUTING.md#lifecycle-of-a-pull-request) under the **Unreleased** section in the repository's `CHANGELOG.md` +1. **DON'T** merge your own pull requests. +If a Repository Maintainer opens a pull request, another Maintainer must merge it + +### Becoming a Repository Maintainer + +(TODO: paste from CM) Repository Maintainers currently consist entirely of Microsoft employees, it's expected that trusted, regular contributors to the PowerShell repository will become maintainers themselves. +Eligibility is heavily dependent on the level of contribution and expertise: individuals who contribute in meaningful ways to the project will be recognized accordingly. + +At any point in time, a Repository Maintainers can nominate a strong community member to become a Repository Maintainer. +Nominations should be submitted in the form of [RFCs][RFC-repo] detailing why that individual is qualified and how they will contribute. +After the RFC has been discussed, a unanimous vote by the PowerShell Committee will be required for the new Repository Maintainer to be confirmed. + +## Area experts + +Area experts are people with knowledge of specific components or technologies in the PowerShell domain. They are responsible for code reviews, issue triage, and providing their expertise to others. + +They have [write access](https://help.github.com/articles/permission-levels-for-an-organization-repository/) to the PowerShell repository which gives them the power to: + +1. `git push` to all branches *except* `master`. +1. Merge pull requests to all branches *except* `master` (though this should not be common given that [`master`is the only long-living branch](../git/powershell-repository-101.md#understand-branches)). +1. Assign labels, milestones, and people to [issues](https://guides.github.com/features/issues/). + +### Area Expert Responsibilities + +If you are an area expert, you are expected to be actively involved in any development, design, or contributions in your area of expertise. + +If you are an Area Expert: + +1. **DO** assign the [correct labels][issue-process] +1. **DO** assign yourself to issues labeled with your area of expertise +1. **DO** [code reviews][TODO] for issues where you're assigned or in your areas of expertise +1. **DO** reply to new issues and pull requests that are related to your area of expertise +(while reviewing PRs, leave your comment even if everything looks good - a simple "Looks good to me" or "LGTM" will suffice, so that we know someone has already taken a look at it). +1. **DO** make sure contributors are following the [contributor guidelines](../../.github/CONTRIBUTING.md). +1. **DO** ask people to resend a pull request, if it [doesn't target `master`](../../.github/CONTRIBUTING.md#lifecycle-of-a-pull-request). +1. **DO** encourage people to [write Pester tests][pester] for all new/changed functionality. +1. **DO** encourage contributors to refer to issues in their pull request description per the [issue template](../../.github/ISSUE_TEMPLATE) (e.g. `Resolves issue #123`) +1. **DO** encourage contributors to create meaningful titles for all PRs. Edit title if necessary. +1. **DO** verify that all contributors are following the [Coding Guidelines](../dev-process/coding-guidelines.md). + +1. **DON'T** create new features, new designs, or change behaviors without following the [RFC][RFC-repo] or approval process + +## Issue Management Process + +See our [Issue Management Process][issue-process] + +## Pull Request Process + +See our [Pull Request Process][pull-request-process] + +[RFC-repo]: https://github.com/PowerShell/PowerShell-RFC +[pester]: ../testing-guidelines/WritingPesterTests.md +[ci-system]: ../testing-guidelines/testing-guidelines.md#ci-system +[breaking-changes]: ../dev-process/breaking-change-contract.md +[issue-process]: ../dev-process/issue-label-descriptions.md +[pull-request-process]: ../dev-process/pull-request-process.md \ No newline at end of file diff --git a/docs/dev-process/pull-request-process.md b/docs/dev-process/pull-request-process.md new file mode 100644 index 000000000..e49210228 --- /dev/null +++ b/docs/dev-process/pull-request-process.md @@ -0,0 +1,47 @@ +# Pull Request Process + +author: Hemant +> Hemant: "SLAs" for pull requests + > ALWAYS point to documents when critiquing PRs + > this should also include the blackbox of Windows/STEX testing + > time can totally be wishy-washy here + > "some tests we can only run internally" + > exact timeline not need for Aug17 + > Windows quality gates + +## Minimum gates (TODO) + +Our [pull request template][pr-template] includes the bare minimum requirements for a pull request to be accepted into PowerShell. This includes: +* Writing tests +* Writing documentation (where does thie one live already? is it where this guidance should exist all up?) +* Our [code review process][code-review] +* Repository maintainer sign-off, per our [governance model][governance] + +## Pull Request Workflow + +1. A contributor opens a pull request. +1. The contributor ensures that their pull request passes the [CI system][ci-system] build. + - If the build fails, a [Repository Maintainer][repository-maintainer] adds the ```waiting for author``` label to the pull request. + The contributor can then continue to update the pull request until the build passes. +1. Once the build passes, the maintainer either reviews the pull request immediately or adds the ```need review``` label. +1. A maintainer or trusted contributor reviews the pull request code. + - If the contributor does not meet the reviewer's standards, the reviewer makes comments. A maintainer then removes the ```need review``` label and adds the ```waiting for author``` label. The contributor must address the comments and repeat from step 2. + - If the contributor meets the reviewer's standards, the reviewer comments that they are satisfied. A maintainer then removes the ```need review``` label. +1. Once the code review is completed, a maintainer merges the pull request. + +### Abandoned Pull Requests +A pull request with the label ```waiting for the author``` for **more than two weeks** without a word from the author is considered abandoned. + +In these cases: + +1. Ping the author of PR to remind him of pending changes. + - If the contributor responds, it's no longer an abandoned pull request, proceed as normal. +2. If the contributor does not respond **within a week**: + - If the reviewer's comments are very minor, merge the change, fix the code immediately, and create a new PR with the fixes addressing the minor comments. + - If the changes required to merge the pull request are significant but needed, create a new branch with the changes and open an issue to merge the code into the dev branch. Mention the original pull request ID in the description of the new issue and close the abandoned pull request. + - If the changes in an abandoned pull request are no longer needed (e.g. due to refactoring of the code base or a design change), simply close the pull request. + +[pr-template]: ../.github/PULL_REQUEST_TEMPLATE.md +[code-review]: code-review-guidelines.md +[governance]: ../community/governance.md +[repository-maintainer]: ../community/governance.md#repository-maintainer \ No newline at end of file diff --git a/docs/maintainers/README.md b/docs/maintainers/README.md index 2c5874a63..67e648074 100644 --- a/docs/maintainers/README.md +++ b/docs/maintainers/README.md @@ -1,6 +1,6 @@ -# PowerShell Maintainers +# Repository Maintainers -Maintainers (a.k.a. coordinators) are trusted people with knowledge in the PowerShell domain. +Repository maintainers are trusted people with knowledge in the PowerShell domain. They have [write access](https://help.github.com/articles/permission-levels-for-an-organization-repository/) to the PowerShell repositories which gives them the power to: From 040c9803f7c2abdc3a9b3dc2a0a747bf79927000 Mon Sep 17 00:00:00 2001 From: joeyaiello Date: Thu, 28 Jul 2016 16:26:14 -0700 Subject: [PATCH 2/6] respond to PR feedback on governance --- docs/community/governance.md | 37 ++++++++++---------- docs/dev-process/issue-label-descriptions.md | 1 + docs/dev-process/pull-request-process.md | 6 ++-- 3 files changed, 23 insertions(+), 21 deletions(-) diff --git a/docs/community/governance.md b/docs/community/governance.md index e89e9de9b..42165140a 100644 --- a/docs/community/governance.md +++ b/docs/community/governance.md @@ -3,7 +3,7 @@ ## Terms * [**PowerShell Committee**](#powershell-committee): A committee of project owners who are responsible for design decisions, approving [RFCs][RFC-repo], and approving new maintainers/committee members -* [**Repository maintainer**](#repository-maintainer): An individual responsible for merging pull requests (PRs) into `master` when all requirements are met (code review, tests, docs, and RFC approval as applicable). +* [**Repository maintainer**](#repository-maintainers): An individual responsible for merging pull requests (PRs) into `master` when all requirements are met (code review, tests, docs, and RFC approval as applicable). Repository Maintainers are the only people with write permissions into `master`. * [**Area experts**](#area-experts): People who are experts for specific components (e.g. PSReadline, the parser) or technologies (e.g. security, performance). Area experts are responsible for code reviews, issue triage, and providing their expertise to others. @@ -15,7 +15,15 @@ The Corporate Maintainer for PowerShell is Microsoft. ## PowerShell Committee -The PowerShell Committee and its members (aka Committee Members) are the primary (TODO: stewards, powerShell experience) decision makers of the PowerShell language, design, and project. +The PowerShell Committee and its members (aka Committee Members) are the primary caretakers of the PowerShell experience, including the PowerShell language, design, and project. + +### Current Committee Members + +* Bruce Payette ([BrucePay](https://github.com/BrucePay)) +* Jason Shirk ([lzybkr](https://github.com/lzybkr)) +* Steve Lee ([SteveL-MSFT](https://github.com/SteveL-MSFT)) +* Hemant Mahawar ([HemantMahawar](https://github.com/HemantMahawar)) +* Joey Aiello ([joeyaiello](https://github.com/joeyaiello)) ### Committee Member Responsibilities @@ -44,7 +52,6 @@ If any Committee Members feels like this behavior is large enough to warrant an As a PowerShell Committee Member: -1. **DO** assign issues to area experts or repository maintainers as appropriate 1. **DO** reply to issues and pull requests with design opinions (this could include offering support for good work or exciting new features) 1. **DO** encourage healthy discussion about the direction of PowerShell @@ -69,20 +76,12 @@ At any point in time, a Committee Member can nominate a strong community member Nominations should be submitted in the form of [RFCs][RFC-repo] detailing why that individual is qualified and how they will contribute. After the RFC has been discussed, a unanimous vote will be required for the new Committee Member to be confirmed. -### Current Committee Members - -* Bruce Payette -* Jason Shirk -* Steve Lee -* Hemant Mahawar -* Joey Aiello ([joeyaiello](https://github.com/joeyaiello)) - ## Repository Maintainers Repository Maintainers are trusted stewards of the PowerShell repository responsible for maintaining consistency and quality of PowerShell code. One of their primary responsibilities is merging pull requests after all requirements have been fulfilled. -Repository Maintainers have [write access](https://help.github.com/articles/permission-levels-for-an-organization-repository/) to the PowerShell repository which gives them the power to: +Repository Maintainers have [write access](https://help.github.com/articles/repository-permission-levels-for-an-organization/) to the PowerShell repository which gives them the power to: 1. Merge pull requests to all branches *including* `master`. 1. `git push` to all branches *including* `master`. @@ -95,7 +94,7 @@ Repository Maintainers enable rapid contributions while maintaining a high level If you are a Repository Maintainer: 1. **DO** add [the correct labels](../dev-process/issue-label-descriptions.md) to issues and pull requests -1. **DO** make sure that [any change requiring approval from the PowerShell Committee](#committee-member-responsibilities) has gone through the proper [RFC][RFC-repo] or approval process +1. **DO** make sure that [any change requiring approval from the PowerShell Committee](#changes-that-require-an-rfc) has gone through the proper [RFC][RFC-repo] or approval process 1. **DO** make sure the correct [Area Experts](#area-experts) are assigned to relevant pull requests and issues. This includes adding extra reviewers when it makes sense (e.g. a pull request that adds remoting capabilities might require a security expert) @@ -126,9 +125,9 @@ At any point in time, a Repository Maintainers can nominate a strong community m Nominations should be submitted in the form of [RFCs][RFC-repo] detailing why that individual is qualified and how they will contribute. After the RFC has been discussed, a unanimous vote by the PowerShell Committee will be required for the new Repository Maintainer to be confirmed. -## Area experts +## Area Experts -Area experts are people with knowledge of specific components or technologies in the PowerShell domain. They are responsible for code reviews, issue triage, and providing their expertise to others. +Area Experts are people with knowledge of specific components or technologies in the PowerShell domain. They are responsible for code reviews, issue triage, and providing their expertise to others. They have [write access](https://help.github.com/articles/permission-levels-for-an-organization-repository/) to the PowerShell repository which gives them the power to: @@ -138,7 +137,7 @@ They have [write access](https://help.github.com/articles/permission-levels-for- ### Area Expert Responsibilities -If you are an area expert, you are expected to be actively involved in any development, design, or contributions in your area of expertise. +If you are an Area Expert, you are expected to be actively involved in any development, design, or contributions in your area of expertise. If you are an Area Expert: @@ -149,7 +148,8 @@ If you are an Area Expert: (while reviewing PRs, leave your comment even if everything looks good - a simple "Looks good to me" or "LGTM" will suffice, so that we know someone has already taken a look at it). 1. **DO** make sure contributors are following the [contributor guidelines](../../.github/CONTRIBUTING.md). 1. **DO** ask people to resend a pull request, if it [doesn't target `master`](../../.github/CONTRIBUTING.md#lifecycle-of-a-pull-request). -1. **DO** encourage people to [write Pester tests][pester] for all new/changed functionality. +1. **DO** ensure that contributors [write Pester tests][pester] for all new/changed functionality +1. **DO** ensure that contributors [write documentation][docs-contributing] for all new-/changed functionality 1. **DO** encourage contributors to refer to issues in their pull request description per the [issue template](../../.github/ISSUE_TEMPLATE) (e.g. `Resolves issue #123`) 1. **DO** encourage contributors to create meaningful titles for all PRs. Edit title if necessary. 1. **DO** verify that all contributors are following the [Coding Guidelines](../dev-process/coding-guidelines.md). @@ -169,4 +169,5 @@ See our [Pull Request Process][pull-request-process] [ci-system]: ../testing-guidelines/testing-guidelines.md#ci-system [breaking-changes]: ../dev-process/breaking-change-contract.md [issue-process]: ../dev-process/issue-label-descriptions.md -[pull-request-process]: ../dev-process/pull-request-process.md \ No newline at end of file +[pull-request-process]: ../dev-process/pull-request-process.md +[docs-contributing]: https://github.com/PowerShell/PowerShell-Docs/blob/staging/CONTRIBUTING.md \ No newline at end of file diff --git a/docs/dev-process/issue-label-descriptions.md b/docs/dev-process/issue-label-descriptions.md index d6349526a..55aff70cc 100644 --- a/docs/dev-process/issue-label-descriptions.md +++ b/docs/dev-process/issue-label-descriptions.md @@ -39,3 +39,4 @@ The assignee(s) are responsible for signing off before the PR will be merged. * `help wanted` : We are looking for someone to work on this issue. * `need review` : This Pull Request is being reviewed. Please see [Pull Request - Code Review](../../.github/CONTRIBUTING.md#pull-request-code-review) +* `waiting for author`: The issue or pull request needs diff --git a/docs/dev-process/pull-request-process.md b/docs/dev-process/pull-request-process.md index e49210228..c3dce379f 100644 --- a/docs/dev-process/pull-request-process.md +++ b/docs/dev-process/pull-request-process.md @@ -4,7 +4,6 @@ author: Hemant > Hemant: "SLAs" for pull requests > ALWAYS point to documents when critiquing PRs > this should also include the blackbox of Windows/STEX testing - > time can totally be wishy-washy here > "some tests we can only run internally" > exact timeline not need for Aug17 > Windows quality gates @@ -24,7 +23,7 @@ Our [pull request template][pr-template] includes the bare minimum requirements - If the build fails, a [Repository Maintainer][repository-maintainer] adds the ```waiting for author``` label to the pull request. The contributor can then continue to update the pull request until the build passes. 1. Once the build passes, the maintainer either reviews the pull request immediately or adds the ```need review``` label. -1. A maintainer or trusted contributor reviews the pull request code. +1. An [Area Expert][area-expert] reviews the pull request code. - If the contributor does not meet the reviewer's standards, the reviewer makes comments. A maintainer then removes the ```need review``` label and adds the ```waiting for author``` label. The contributor must address the comments and repeat from step 2. - If the contributor meets the reviewer's standards, the reviewer comments that they are satisfied. A maintainer then removes the ```need review``` label. 1. Once the code review is completed, a maintainer merges the pull request. @@ -44,4 +43,5 @@ In these cases: [pr-template]: ../.github/PULL_REQUEST_TEMPLATE.md [code-review]: code-review-guidelines.md [governance]: ../community/governance.md -[repository-maintainer]: ../community/governance.md#repository-maintainer \ No newline at end of file +[repository-maintainer]: ../community/governance.md#repository-maintainers +[area-expert]: ../community/governance.md#area-experts#area-experts \ No newline at end of file From d9b1093a1c3eaeed828ce1b1a96f38e9a6e4b7b6 Mon Sep 17 00:00:00 2001 From: joeyaiello Date: Tue, 2 Aug 2016 15:55:32 -0700 Subject: [PATCH 3/6] address offline feedback for governance --- docs/community/governance.md | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/docs/community/governance.md b/docs/community/governance.md index 42165140a..7a2ac98ca 100644 --- a/docs/community/governance.md +++ b/docs/community/governance.md @@ -3,14 +3,18 @@ ## Terms * [**PowerShell Committee**](#powershell-committee): A committee of project owners who are responsible for design decisions, approving [RFCs][RFC-repo], and approving new maintainers/committee members +* **Project Leads**: Project Leads supports the PowerShell Committee, engineering teams, and community by communicating with other Microsoft teams and leadership as well as other companies to resolve disputes. +They also have optional votes on the PowerShell Committee when they choose to invoke them. * [**Repository maintainer**](#repository-maintainers): An individual responsible for merging pull requests (PRs) into `master` when all requirements are met (code review, tests, docs, and RFC approval as applicable). Repository Maintainers are the only people with write permissions into `master`. * [**Area experts**](#area-experts): People who are experts for specific components (e.g. PSReadline, the parser) or technologies (e.g. security, performance). Area experts are responsible for code reviews, issue triage, and providing their expertise to others. +* **Corporation**: The Corporation owns the PowerShell repository and, under extreme circumstances, reserves the right to dissolve or reform the PowerShell Committee. +The Corporation for PowerShell is Microsoft. * **Corporate Maintainer**: The Corporate Maintainer is an entity, person or set of persons, with the ability to veto decisions made by the PowerShell Committee or any other collaborators on the PowerShell project. This veto power will be used with restraint since it is intended that the community drive the project. -Under extreme circumstances the Corporate Maintainer also reserves the right to dissolve or reform the PowerShell Committee. -The Corporate Maintainer for PowerShell is Microsoft. +The Corporate Maintainer is determined by the Corporation both initially and in continuation. +The initial Corporate Maintainer for PowerShell is Jeffrey Snover. * [**RFC process**][RFC-repo]: The "review-for-comment" (RFC) process whereby design decisions get made. ## PowerShell Committee @@ -87,6 +91,14 @@ Repository Maintainers have [write access](https://help.github.com/articles/repo 1. `git push` to all branches *including* `master`. 1. Correctly assigning labels, milestones, and contributors to [issues](https://guides.github.com/features/issues/) +### Current Repository Maintainers + +* Sergei Vorobev ([vors](https://github.com/vors)) +* Jason Shirk ([lzybkr](https://github.com/lzybkr)) +* Dongbo Wang ([daxian-dbw](https://github.com/daxian-dbw)) +* Travis Plunk ([TravisEz123](https://github.com/TravisEz123)) +* Mike Richmond ([mirichmo](https://github.com/mirichmo)) + ### Repository Maintainer Responsibilities Repository Maintainers enable rapid contributions while maintaining a high level of quality in PowerShell by ensuring that all development processes are being followed correctly. From 42019e511e3f4afac631305137108e0528419cde Mon Sep 17 00:00:00 2001 From: joeyaiello Date: Wed, 3 Aug 2016 11:06:36 -0700 Subject: [PATCH 4/6] refactor maintainer and governance docs Now that we've gotten enough of a sign-off from everyone involved in the governance process, the docs need to be reworked to use a consistent terminology set, links, and directory structure. --- .github/CONTRIBUTING.md | 13 +-- docs/community/governance.md | 61 +--------- docs/maintainers/README.md | 107 ++++++++++++------ docs/maintainers/issue-management-process.md | 12 -- .../issue-management.md} | 6 +- .../pull-request-process.md | 10 -- docs/maintainers/pull-request-rules.md | 18 --- 7 files changed, 90 insertions(+), 137 deletions(-) delete mode 100644 docs/maintainers/issue-management-process.md rename docs/{dev-process/issue-label-descriptions.md => maintainers/issue-management.md} (90%) rename docs/{dev-process => maintainers}/pull-request-process.md (90%) delete mode 100644 docs/maintainers/pull-request-rules.md diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 82b760bd5..3d888b5a0 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -29,18 +29,18 @@ Quick Start Checklist Contributing to Issues ---------------------- -* Review the [Issue Label Descriptions](../docs/dev-process/issue-label-descriptions.md). +* Review [Issue Management][issue-management]. * Check if the issue you are going to file already exists in our [GitHub issues][open-issue]. * If you can't find your issue already, [open a new issue](https://github.com/PowerShell/PowerShell/issues/new), making sure to follow the directions as best you can. -* If the issue is marked as [`Help Wanted`][help-wanted-issue], +* If the issue is marked as [`help wanted`][help-wanted-issue], the PowerShell maintainers are looking for help with the issue. Contributing to Documentation ----------------------------- -### Contributing to documentation related to the PowerShell the product +### Contributing to documentation related to PowerShell Please see the [Contributor Guide in `PowerShell/PowerShell-Docs`](https://github.com/PowerShell/PowerShell-Docs/blob/staging/CONTRIBUTING.md). @@ -86,9 +86,8 @@ Additional references: ![Github-PR-dev.png](Images/Github-PR-dev.png) -* If your contribution in a way that changes the user or developer experience, - you are expected to document those changes. - See [Contributing to documentation related to the PowerShell the product](#contributing-to-documentation-related-to-the-powershell-the-product). +* If you're contributing in a way that changes the user or developer experience, you are expected to document those changes. +See [Contributing to documentation related to PowerShell](#contributing-to-documentation-related-to-powershell). * Add a meaningful title of the PR describing what change you want to check in. Don't simply put: "Fixes issue #5". @@ -251,7 +250,7 @@ Once you sign a CLA, all your existing and future pull requests will be labeled [testing-guidelines]: ../docs/testing-guidelines/testing-guidelines.md [running-tests-outside-of-ci]: ../docs/testing-guidelines/testing-guidelines.md#running-tests-outside-of-ci -[issue-triage]: ../docs/dev-process/issue-management-process.md +[issue-management]: ../docs/maintainers/issue-management.md [governance]: ../docs/community/governance.md [using-prs]: https://help.github.com/articles/using-pull-requests/ [fork-a-repo]: https://help.github.com/articles/fork-a-repo/ diff --git a/docs/community/governance.md b/docs/community/governance.md index 7a2ac98ca..bb2ccfedd 100644 --- a/docs/community/governance.md +++ b/docs/community/governance.md @@ -3,13 +3,13 @@ ## Terms * [**PowerShell Committee**](#powershell-committee): A committee of project owners who are responsible for design decisions, approving [RFCs][RFC-repo], and approving new maintainers/committee members -* **Project Leads**: Project Leads supports the PowerShell Committee, engineering teams, and community by communicating with other Microsoft teams and leadership as well as other companies to resolve disputes. +* **Project Leads**: Project Leads support the PowerShell Committee, engineering teams, and community by working across Microsoft teams and leadership, and working through industry issues with other companies. They also have optional votes on the PowerShell Committee when they choose to invoke them. * [**Repository maintainer**](#repository-maintainers): An individual responsible for merging pull requests (PRs) into `master` when all requirements are met (code review, tests, docs, and RFC approval as applicable). Repository Maintainers are the only people with write permissions into `master`. * [**Area experts**](#area-experts): People who are experts for specific components (e.g. PSReadline, the parser) or technologies (e.g. security, performance). Area experts are responsible for code reviews, issue triage, and providing their expertise to others. -* **Corporation**: The Corporation owns the PowerShell repository and, under extreme circumstances, reserves the right to dissolve or reform the PowerShell Committee. +* **Corporation**: The Corporation owns the PowerShell repository and, under extreme circumstances, reserves the right to dissolve or reform the PowerShell Committee, the Project Leads, and the Corporate Maintainer. The Corporation for PowerShell is Microsoft. * **Corporate Maintainer**: The Corporate Maintainer is an entity, person or set of persons, with the ability to veto decisions made by the PowerShell Committee or any other collaborators on the PowerShell project. This veto power will be used with restraint since it is intended that the community drive the project. @@ -83,59 +83,9 @@ After the RFC has been discussed, a unanimous vote will be required for the new ## Repository Maintainers Repository Maintainers are trusted stewards of the PowerShell repository responsible for maintaining consistency and quality of PowerShell code. -One of their primary responsibilities is merging pull requests after all requirements have been fulfilled. +One of their primary responsibilities is merging pull requests after all requirements have been fulfilled. -Repository Maintainers have [write access](https://help.github.com/articles/repository-permission-levels-for-an-organization/) to the PowerShell repository which gives them the power to: - -1. Merge pull requests to all branches *including* `master`. -1. `git push` to all branches *including* `master`. -1. Correctly assigning labels, milestones, and contributors to [issues](https://guides.github.com/features/issues/) - -### Current Repository Maintainers - -* Sergei Vorobev ([vors](https://github.com/vors)) -* Jason Shirk ([lzybkr](https://github.com/lzybkr)) -* Dongbo Wang ([daxian-dbw](https://github.com/daxian-dbw)) -* Travis Plunk ([TravisEz123](https://github.com/TravisEz123)) -* Mike Richmond ([mirichmo](https://github.com/mirichmo)) - -### Repository Maintainer Responsibilities - -Repository Maintainers enable rapid contributions while maintaining a high level of quality in PowerShell by ensuring that all development processes are being followed correctly. - -If you are a Repository Maintainer: - -1. **DO** add [the correct labels](../dev-process/issue-label-descriptions.md) to issues and pull requests -1. **DO** make sure that [any change requiring approval from the PowerShell Committee](#changes-that-require-an-rfc) has gone through the proper [RFC][RFC-repo] or approval process -1. **DO** make sure the correct [Area Experts](#area-experts) are assigned to relevant pull requests and issues. -This includes adding extra reviewers when it makes sense -(e.g. a pull request that adds remoting capabilities might require a security expert) -1. **DO** validate that code reviews have been performed before merging a pull request -1. **DO** validate that applicable tests and documentation have been written before merging a pull request -1. **DO** make sure contributors are following the [contributor guidelines](../../.github/CONTRIBUTING.md) -1. **DO** ask people to resend a pull request, if it [doesn't target `master`](../../.github/CONTRIBUTING.md#lifecycle-of-a-pull-request). -1. **DO** wait for the [CI system][ci-system] build to pass for pull requests. -1. **DO** encourage contributors to refer to issues in their pull request description per the [issue template](../../.github/ISSUE_TEMPLATE) (e.g. `Resolves issue #123`) -1. **DO** encourage contributors to create meaningful titles for all PRs. -Edit the title if necessary to provide clarity on the problem -1. **DO** verify that all contributors are following the [Coding Guidlines](../dev-process/coding-guidelines.md) -1. **DO** ensure that each contributor has signed a valid Contributor License Agreement (CLA) -1. **DO** verify compliance with any third party code license terms (e.g., requiring attribution, etc.) if the contribution contains third party code - -1. **DON'T** merge pull requests with a failed CI build into `master` -1. **DON'T** merge pull requests without the label `cla-signed` or `cla-not-required` from the Microsoft CLA bot -1. **DON'T** merge pull requests that do not [include all meaningful changes](../../.github/CONTRIBUTING.md#lifecycle-of-a-pull-request) under the **Unreleased** section in the repository's `CHANGELOG.md` -1. **DON'T** merge your own pull requests. -If a Repository Maintainer opens a pull request, another Maintainer must merge it - -### Becoming a Repository Maintainer - -(TODO: paste from CM) Repository Maintainers currently consist entirely of Microsoft employees, it's expected that trusted, regular contributors to the PowerShell repository will become maintainers themselves. -Eligibility is heavily dependent on the level of contribution and expertise: individuals who contribute in meaningful ways to the project will be recognized accordingly. - -At any point in time, a Repository Maintainers can nominate a strong community member to become a Repository Maintainer. -Nominations should be submitted in the form of [RFCs][RFC-repo] detailing why that individual is qualified and how they will contribute. -After the RFC has been discussed, a unanimous vote by the PowerShell Committee will be required for the new Repository Maintainer to be confirmed. +For more information on Repository Maintainers--their responsibilities, who they are, and how one becomes a Maintainer--see the [README for Repository Maintainers][maintainers]. ## Area Experts @@ -182,4 +132,5 @@ See our [Pull Request Process][pull-request-process] [breaking-changes]: ../dev-process/breaking-change-contract.md [issue-process]: ../dev-process/issue-label-descriptions.md [pull-request-process]: ../dev-process/pull-request-process.md -[docs-contributing]: https://github.com/PowerShell/PowerShell-Docs/blob/staging/CONTRIBUTING.md \ No newline at end of file +[docs-contributing]: https://github.com/PowerShell/PowerShell-Docs/blob/staging/CONTRIBUTING.md +[maintainers]: ../maintainers/README.md \ No newline at end of file diff --git a/docs/maintainers/README.md b/docs/maintainers/README.md index 67e648074..c7a0968b3 100644 --- a/docs/maintainers/README.md +++ b/docs/maintainers/README.md @@ -1,59 +1,86 @@ # Repository Maintainers -Repository maintainers are trusted people with knowledge in the PowerShell domain. +Repository Maintainers are trusted stewards of the PowerShell repository responsible for maintaining consistency and quality of PowerShell code. +One of their primary responsibilities is merging pull requests after all requirements have been fulfilled. They have [write access](https://help.github.com/articles/permission-levels-for-an-organization-repository/) to the PowerShell repositories which gives them the power to: -1. `push`. -2. Merge pull requests. -3. Assign labels, milestones, and people to [issues](https://guides.github.com/features/issues/). +1. `git push` to the official PowerShell repository +2. Merge pull requests +3. Assign labels, milestones, and people to [issues](https://guides.github.com/features/issues/) ## Table of Contents -- [Rules](#rules) +- [Current Repository Maintainers](#current-repository-maintainers) +- [Repository Maintainer Responsibilities](#repository-maintainer-responsibilities) - [Issue Management Process](#issue-management-process) - [Pull Request Workflow](#pull-management-process) - [Abandoned Pull Requests](#abandoned-pull-requests) +- [Becoming a Repository Maintainer](#becoming-a-repository-maintainer) -## Rules +## Current Repository Maintainers -If you are a maintainer, please follow these rules: +* Sergei Vorobev ([vors](https://github.com/vors)) +* Jason Shirk ([lzybkr](https://github.com/lzybkr)) +* Dongbo Wang ([daxian-dbw](https://github.com/daxian-dbw)) +* Travis Plunk ([TravisEz123](https://github.com/TravisEz123)) +* Mike Richmond ([mirichmo](https://github.com/mirichmo)) +* Andy Schwartzmeyer ([andschwa](https://github.com/andschwa)) -1. **DO** reply to new issues and pull requests (while reviewing PRs, leave your comment even if everything looks good - simple "Looks good to me" or "LGTM" will suffice, so that we know someone has already taken a look at it). -1. **DO** make sure contributors are following the [contributor guidelines](../../.github/CONTRIBUTING.md). -1. **DO** ask people to resend a pull request, if it targets [the wrong branch](../../.github/CONTRIBUTING.md#lifecycle-of-a-pull-request). -1. **DO** encourage people to write Pester tests for all new/changed functionality. -1. **DO** wait for the [CI system][ci-system] build to pass for pull requests. -1. **DO** encourage contributors to refer to issues in PR title/description (e.g. ```Closes #11```). Edit title if necessary. -1. **DO** encourage contributors to create meaningful titles for all PRs. Edit title if necessary. -1. **DO** verify that all contributors are following the [coding guidelines](../dev-process/coding-guidelines.md). -1. **DO** ensure that each contributor has signed a valid Contributor License Agreement (CLA). -1. **DO** verify compliance with any third party code license terms (e.g., requiring attribution, etc.) if the contribution contains third party code. +## Repository Maintainer Responsibilities -1. **DON'T** merge pull requests with a failed CI build. -1. **DON'T** merge pull requests without the label `cla-signed` or `cla-not-required` from the Microsoft CLA bot. -1. **DON'T** merge pull requests that do not [include all meaningful changes](../../.github/CONTRIBUTING.md#lifecycle-of-a-pull-request) under the **Unreleased** section in the repository's `CHANGELOG.md`. -1. **DON'T** merge your own pull requests before they are reviewed by someone else. - - If there is **no one** else to review your pull request, please wait **24** hours to merge it in case anyone comes along and has a comment. +Repository Maintainers enable rapid contributions while maintaining a high level of quality in PowerShell by ensuring that all development processes are being followed correctly. + +If you are a Repository Maintainer, you: + +1. **MUST** ensure that each contributor has signed a valid Contributor License Agreement (CLA) +1. **MUST** verify compliance with any third party code license terms (e.g., requiring attribution, etc.) if the contribution contains third party code. +1. **MUST** make sure that [any change requiring approval from the PowerShell Committee](#changes-that-require-an-rfc) has gone through the proper [RFC][RFC-repo] or approval process +1. **MUST** validate that code reviews have been conducted before merging a pull request when no code is written +1. **MUST** validate that tests and documentation have been written before merging a pull request that contains new functionality +1. **SHOULD** add [the correct labels][issue-management] to issues and pull requests +1. **SHOULD** make sure the correct [Area Experts](#area-experts) are assigned to relevant pull requests and issues. +This includes adding extra reviewers when it makes sense +(e.g. a pull request that adds remoting capabilities might require a security expert) +1. **SHOULD** validate that the names and email addresses in the git commits reasonably match identity of the person submitting the pull request +1. **SHOULD** make sure contributors are following the [contributor guidelines][CONTRIBUTING] +1. **SHOULD** ask people to resend a pull request, if it [doesn't target `master`](../../.github/CONTRIBUTING.md#lifecycle-of-a-pull-request) +1. **SHOULD** wait for the [CI system][ci-system] build to pass for pull requests +(unless, for instance, the pull request is being submitted to fix broken CI) +1. **SHOULD** encourage contributors to refer to issues in their pull request description per the [issue template](../../.github/ISSUE_TEMPLATE) (e.g. `Resolves issue #123`) +1. **SHOULD** encourage contributors to create meaningful titles for all PRs. +Edit the title if necessary to provide clarity on the problem +1. **SHOULD** encourage contributes to write meaningful, descriptive git commits +1. **SHOULD NOT** merge pull requests with a failed CI build +(unless, for instance, the pull request is being submitted to fix broken CI) +1. **SHOULD NOT** merge pull requests without the label `cla-signed` or `cla-not-required` from the Microsoft CLA bot +(unless the CLA bot is broken, and CLA signing can be confirmed through other means) +1. **SHOULD NOT** merge pull requests too quickly after they're submitted. +Even if the pull request meets all the requirements, people should have time to give their input +(unless the pull request is particularly urgent for some reason) +1. **SHOULD NOT** merge your own pull requests. +If a Repository Maintainer opens a pull request, another Maintainer should merge it unless there are extreme, short-term circumstances requiring a merge or another Maintainer has given explicit sign-off without merging ## Issue Management Process -Please see [Issue Management Process](./issue-management-process.md) +Please see [Issue Management][issue-management] ## Pull Request Workflow + 1. A contributor opens a pull request. -2. The contributor ensures that their pull request passes the [CI system][ci-system] build. +1. The contributor ensures that their pull request passes the [CI system][ci-system] build. - If the build fails, a maintainer adds the ```waiting for author``` label to the pull request. - The contributor can then continue to update the pull request until the build passes. -2. Once the build passes, the maintainer either reviews the pull request immediately or adds the ```need review``` label. -3. A maintainer or trusted contributor reviews the pull request code. + The contributor can then continue to update the pull request until the build passes. +1. Once the build passes, the maintainer either reviews the pull request immediately or adds the ```need review``` label. +1. A maintainer or trusted contributor reviews the pull request code. - If the contributor does not meet the reviewer's standards, the reviewer makes comments. - A maintainer then removes the ```need review``` label and adds the ```waiting for author``` label. - The contributor must address the comments and repeat from step 2. + A maintainer then removes the ```need review``` label and adds the ```waiting for author``` label. + The contributor must address the comments and repeat from step 2. - If the contributor meets the reviewer's standards, the reviewer comments that they are satisfied. - A maintainer then removes the ```need review``` label. -3. Once the code review is completed, a maintainer merges the pull request. + A maintainer then removes the ```need review``` label. +1. Once the code review is completed, a maintainer merges the pull request. ### Abandoned Pull Requests + A pull request with the label ```waiting for the author``` for **more than two weeks** without a word from the author is considered abandoned. In these cases: @@ -61,9 +88,21 @@ In these cases: 1. Ping the author of PR to remind him of pending changes. - If the contributor responds, it's no longer an abandoned pull request, proceed as normal. 2. If the contributor does not respond **within a week**: - - If the reviewer's comments are very minor, merge the change, fix the code immediately, and create a new PR with the fixes addressing the minor comments. - - If the changes required to merge the pull request are significant but needed, create a new branch with the changes and open an issue to merge the code into the dev branch. - Mention the original pull request ID in the description of the new issue and close the abandoned pull request. + - Create a new branch with the changes and open an issue to merge the code into the dev branch. + Mention the original pull request ID in the description of the new issue and close the abandoned pull request. - If the changes in an abandoned pull request are no longer needed (e.g. due to refactoring of the code base or a design change), simply close the pull request. +## Becoming a Repository Maintainer + +Repository Maintainers currently consist entirely of Microsoft employees +It is expected that over time, regular trusted contributors to the PowerShell repository will be made Repository Maintainers. +Eligibility is heavily dependent on the level of contribution and expertise: individuals who contribute in meaningful ways to the project will be recognized accordingly. + +At any point in time, a Repository Maintainers can nominate a strong community member to become a Repository Maintainer. +Nominations should be submitted in the form of [RFCs][RFC-repo] detailing why that individual is qualified and how they will contribute. +After the RFC has been discussed, a unanimous vote by the PowerShell Committee will be required for the new Repository Maintainer to be confirmed. + +[RFC-repo]: https://github.com/PowerShell/PowerShell-RFC [ci-system]: ../testing-guidelines/testing-guidelines.md#ci-system +[issue-management]: issue-management.md +[CONTRIBUTING]: ../../.github/CONTRIBUTING.MD \ No newline at end of file diff --git a/docs/maintainers/issue-management-process.md b/docs/maintainers/issue-management-process.md deleted file mode 100644 index dde111f27..000000000 --- a/docs/maintainers/issue-management-process.md +++ /dev/null @@ -1,12 +0,0 @@ -author: Jason/Andy - - -> meaning of label, assignees, etc. -> triage process -> requirements for resolving (timing), closing... - -# Maintainer - Issue Management Process - -## Issue Label Descriptions - -See [Issue Label Descriptions](../dev-process/issue-label-descriptions) diff --git a/docs/dev-process/issue-label-descriptions.md b/docs/maintainers/issue-management.md similarity index 90% rename from docs/dev-process/issue-label-descriptions.md rename to docs/maintainers/issue-management.md index 55aff70cc..bb2091e18 100644 --- a/docs/dev-process/issue-label-descriptions.md +++ b/docs/maintainers/issue-management.md @@ -1,3 +1,5 @@ +# Issue Management + ## Long-living issue labels ### Feature areas @@ -38,5 +40,7 @@ Issues can be in one of the following states: The assignee(s) are responsible for signing off before the PR will be merged. * `help wanted` : We are looking for someone to work on this issue. -* `need review` : This Pull Request is being reviewed. Please see [Pull Request - Code Review](../../.github/CONTRIBUTING.md#pull-request-code-review) +* `need review` : This pull request is being reviewed. Please see [Pull Request - Code Review](../../.github/CONTRIBUTING.md#pull-request-code-review) * `waiting for author`: The issue or pull request needs +* `add to changelog`: The PR requires an addition to the changelog. +Should be removed when it has been added. diff --git a/docs/dev-process/pull-request-process.md b/docs/maintainers/pull-request-process.md similarity index 90% rename from docs/dev-process/pull-request-process.md rename to docs/maintainers/pull-request-process.md index c3dce379f..04acca970 100644 --- a/docs/dev-process/pull-request-process.md +++ b/docs/maintainers/pull-request-process.md @@ -1,15 +1,5 @@ # Pull Request Process -author: Hemant -> Hemant: "SLAs" for pull requests - > ALWAYS point to documents when critiquing PRs - > this should also include the blackbox of Windows/STEX testing - > "some tests we can only run internally" - > exact timeline not need for Aug17 - > Windows quality gates - -## Minimum gates (TODO) - Our [pull request template][pr-template] includes the bare minimum requirements for a pull request to be accepted into PowerShell. This includes: * Writing tests * Writing documentation (where does thie one live already? is it where this guidance should exist all up?) diff --git a/docs/maintainers/pull-request-rules.md b/docs/maintainers/pull-request-rules.md deleted file mode 100644 index 0ac149e4d..000000000 --- a/docs/maintainers/pull-request-rules.md +++ /dev/null @@ -1,18 +0,0 @@ -author: Hemant -> Hemant: "SLAs" for pull requests - > ALWAYS point to documents when critiquing PRs - > this should also include the blackbox of Windows/STEX testing - > time can totally be wishy-washy here - > "some tests we can only run internally" - > exact timeline not need for Aug17 - > Windows quality gates - -## Minimum gates (TODO) - -Our [pull request template][pr-template] includes the bare minimum requirements for a pull request to be accepted into PowerShell. This includes: -* Writing tests -* Writing documentation (where does thie one live already? is it where this guidance should exist all up?) -* Repository maintainer sign-off, per our [governance model][governance] - -[pr-template]: ../../.github/PULL_REQUEST_TEMPLATE.md -[governance]: ../community/governance.md From c9ae0c348c13619a0fa8f7b0c97ad0e588516f9d Mon Sep 17 00:00:00 2001 From: joeyaiello Date: Thu, 4 Aug 2016 14:01:40 -0700 Subject: [PATCH 5/6] fix broken links in governance PR --- docs/community/governance.md | 8 ++++---- docs/maintainers/pull-request-process.md | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/community/governance.md b/docs/community/governance.md index bb2ccfedd..480dbdae5 100644 --- a/docs/community/governance.md +++ b/docs/community/governance.md @@ -94,7 +94,7 @@ Area Experts are people with knowledge of specific components or technologies in They have [write access](https://help.github.com/articles/permission-levels-for-an-organization-repository/) to the PowerShell repository which gives them the power to: 1. `git push` to all branches *except* `master`. -1. Merge pull requests to all branches *except* `master` (though this should not be common given that [`master`is the only long-living branch](../git/powershell-repository-101.md#understand-branches)). +1. Merge pull requests to all branches *except* `master` (though this should not be common given that [`master`is the only long-living branch](../git/README.md#understand-branches)). 1. Assign labels, milestones, and people to [issues](https://guides.github.com/features/issues/). ### Area Expert Responsibilities @@ -112,7 +112,7 @@ If you are an Area Expert: 1. **DO** ask people to resend a pull request, if it [doesn't target `master`](../../.github/CONTRIBUTING.md#lifecycle-of-a-pull-request). 1. **DO** ensure that contributors [write Pester tests][pester] for all new/changed functionality 1. **DO** ensure that contributors [write documentation][docs-contributing] for all new-/changed functionality -1. **DO** encourage contributors to refer to issues in their pull request description per the [issue template](../../.github/ISSUE_TEMPLATE) (e.g. `Resolves issue #123`) +1. **DO** encourage contributors to refer to issues in their pull request description per the [issue template](../../.github/ISSUE_TEMPLATE.md) (e.g. `Resolves issue #123`) 1. **DO** encourage contributors to create meaningful titles for all PRs. Edit title if necessary. 1. **DO** verify that all contributors are following the [Coding Guidelines](../dev-process/coding-guidelines.md). @@ -130,7 +130,7 @@ See our [Pull Request Process][pull-request-process] [pester]: ../testing-guidelines/WritingPesterTests.md [ci-system]: ../testing-guidelines/testing-guidelines.md#ci-system [breaking-changes]: ../dev-process/breaking-change-contract.md -[issue-process]: ../dev-process/issue-label-descriptions.md -[pull-request-process]: ../dev-process/pull-request-process.md +[issue-process]: ../maintainers/issue-management.md +[pull-request-process]: ../maintainers/pull-request-process.md [docs-contributing]: https://github.com/PowerShell/PowerShell-Docs/blob/staging/CONTRIBUTING.md [maintainers]: ../maintainers/README.md \ No newline at end of file diff --git a/docs/maintainers/pull-request-process.md b/docs/maintainers/pull-request-process.md index 04acca970..63fb7ead8 100644 --- a/docs/maintainers/pull-request-process.md +++ b/docs/maintainers/pull-request-process.md @@ -30,7 +30,7 @@ In these cases: - If the changes required to merge the pull request are significant but needed, create a new branch with the changes and open an issue to merge the code into the dev branch. Mention the original pull request ID in the description of the new issue and close the abandoned pull request. - If the changes in an abandoned pull request are no longer needed (e.g. due to refactoring of the code base or a design change), simply close the pull request. -[pr-template]: ../.github/PULL_REQUEST_TEMPLATE.md +[pr-template]: ../../.github/PULL_REQUEST_TEMPLATE.md [code-review]: code-review-guidelines.md [governance]: ../community/governance.md [repository-maintainer]: ../community/governance.md#repository-maintainers From 580c7b78529c88ab39c6fa11b6a8655cfd864789 Mon Sep 17 00:00:00 2001 From: joeyaiello Date: Tue, 9 Aug 2016 16:42:34 -0700 Subject: [PATCH 6/6] address final PR/offline feedback for governance --- .github/CONTRIBUTING.md | 2 +- docs/community/governance.md | 4 ++-- docs/maintainers/README.md | 6 ++++-- docs/maintainers/pull-request-process.md | 10 +++++----- 4 files changed, 12 insertions(+), 10 deletions(-) diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 3d888b5a0..59db52bb8 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -44,7 +44,7 @@ Contributing to Documentation Please see the [Contributor Guide in `PowerShell/PowerShell-Docs`](https://github.com/PowerShell/PowerShell-Docs/blob/staging/CONTRIBUTING.md). -### Contributing to documentation related to contributing or maintaining the PowerShell Project +### Contributing to documentation related to maintaining or contributing to the PowerShell project * When writing Markdown documentation, use [semantic linefeeds][]. In most cases, it means "once clause / idea per line". diff --git a/docs/community/governance.md b/docs/community/governance.md index 480dbdae5..07f5e44bf 100644 --- a/docs/community/governance.md +++ b/docs/community/governance.md @@ -105,14 +105,14 @@ If you are an Area Expert: 1. **DO** assign the [correct labels][issue-process] 1. **DO** assign yourself to issues labeled with your area of expertise -1. **DO** [code reviews][TODO] for issues where you're assigned or in your areas of expertise +1. **DO** code reviews for issues where you're assigned or in your areas of expertise. 1. **DO** reply to new issues and pull requests that are related to your area of expertise (while reviewing PRs, leave your comment even if everything looks good - a simple "Looks good to me" or "LGTM" will suffice, so that we know someone has already taken a look at it). 1. **DO** make sure contributors are following the [contributor guidelines](../../.github/CONTRIBUTING.md). 1. **DO** ask people to resend a pull request, if it [doesn't target `master`](../../.github/CONTRIBUTING.md#lifecycle-of-a-pull-request). 1. **DO** ensure that contributors [write Pester tests][pester] for all new/changed functionality 1. **DO** ensure that contributors [write documentation][docs-contributing] for all new-/changed functionality -1. **DO** encourage contributors to refer to issues in their pull request description per the [issue template](../../.github/ISSUE_TEMPLATE.md) (e.g. `Resolves issue #123`) +1. **DO** encourage contributors to refer to issues in their pull request description per the [pull request template](../../.github/PULL_REQUEST_TEMPLATE.md) (e.g. `Resolves issue #123`) 1. **DO** encourage contributors to create meaningful titles for all PRs. Edit title if necessary. 1. **DO** verify that all contributors are following the [Coding Guidelines](../dev-process/coding-guidelines.md). diff --git a/docs/maintainers/README.md b/docs/maintainers/README.md index c7a0968b3..16d7a6ca7 100644 --- a/docs/maintainers/README.md +++ b/docs/maintainers/README.md @@ -46,7 +46,9 @@ This includes adding extra reviewers when it makes sense 1. **SHOULD** ask people to resend a pull request, if it [doesn't target `master`](../../.github/CONTRIBUTING.md#lifecycle-of-a-pull-request) 1. **SHOULD** wait for the [CI system][ci-system] build to pass for pull requests (unless, for instance, the pull request is being submitted to fix broken CI) -1. **SHOULD** encourage contributors to refer to issues in their pull request description per the [issue template](../../.github/ISSUE_TEMPLATE) (e.g. `Resolves issue #123`) +1. **SHOULD** encourage contributors to refer to issues in their pull request description per the [pull request template](../../.github/PULL_REQUEST_TEMPLATE.md) (e.g. `Resolves issue #123`). +If a user did not create an issue prior to submitting their pull request, their pull request should not be rejected. +However, they should be reminded to create an issue in the future to frontload any potential problems with the work and to minimize duplication of efforts. 1. **SHOULD** encourage contributors to create meaningful titles for all PRs. Edit the title if necessary to provide clarity on the problem 1. **SHOULD** encourage contributes to write meaningful, descriptive git commits @@ -105,4 +107,4 @@ After the RFC has been discussed, a unanimous vote by the PowerShell Committee w [RFC-repo]: https://github.com/PowerShell/PowerShell-RFC [ci-system]: ../testing-guidelines/testing-guidelines.md#ci-system [issue-management]: issue-management.md -[CONTRIBUTING]: ../../.github/CONTRIBUTING.MD \ No newline at end of file +[CONTRIBUTING]: ../../.github/CONTRIBUTING.md \ No newline at end of file diff --git a/docs/maintainers/pull-request-process.md b/docs/maintainers/pull-request-process.md index 63fb7ead8..82799e9f0 100644 --- a/docs/maintainers/pull-request-process.md +++ b/docs/maintainers/pull-request-process.md @@ -10,16 +10,16 @@ Our [pull request template][pr-template] includes the bare minimum requirements 1. A contributor opens a pull request. 1. The contributor ensures that their pull request passes the [CI system][ci-system] build. - - If the build fails, a [Repository Maintainer][repository-maintainer] adds the ```waiting for author``` label to the pull request. + - If the build fails, a [Repository Maintainer][repository-maintainer] adds the `Review - waiting on author` label to the pull request. The contributor can then continue to update the pull request until the build passes. -1. Once the build passes, the maintainer either reviews the pull request immediately or adds the ```need review``` label. +1. Once the build passes, the maintainer either reviews the pull request immediately or adds the `Review - needed` label. 1. An [Area Expert][area-expert] reviews the pull request code. - - If the contributor does not meet the reviewer's standards, the reviewer makes comments. A maintainer then removes the ```need review``` label and adds the ```waiting for author``` label. The contributor must address the comments and repeat from step 2. - - If the contributor meets the reviewer's standards, the reviewer comments that they are satisfied. A maintainer then removes the ```need review``` label. + - If the contributor does not meet the reviewer's standards, the reviewer makes comments. A maintainer then removes the `Review - needed` label and adds the `Review - waiting on author` label. The contributor must address the comments and repeat from step 2. + - If the contributor meets the reviewer's standards, the reviewer comments that they are satisfied. A maintainer then removes the `need review` label. 1. Once the code review is completed, a maintainer merges the pull request. ### Abandoned Pull Requests -A pull request with the label ```waiting for the author``` for **more than two weeks** without a word from the author is considered abandoned. +A pull request with the label `Review - waiting on author` for **more than two weeks** without a word from the author is considered abandoned. In these cases: