Skip to content

Commit

Permalink
Add pull request acceptance criteria (dotnet#732)
Browse files Browse the repository at this point in the history
  • Loading branch information
ryalanms authored May 23, 2019
1 parent a70c474 commit aa562e5
Show file tree
Hide file tree
Showing 3 changed files with 107 additions and 14 deletions.
99 changes: 99 additions & 0 deletions Documentation/acceptance_criteria.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
# Pull Request Acceptance Criteria

Thank you for contributing to **WPF for .NET Core!** We ask that before you start work on a feature that you would like to contribute, please file an issue describing your proposed change: We will be happy to work with you to figure out the best approach, provide guidance and mentorship throughout feature development, and help avoid any wasted or duplicate effort.

#### What types of pull requests will be accepted before WPF for .NET Core General Availability in September?

WPF for .NET Core is seeking functional and performance parity with .NET Framework to offer customers a direct porting path from .NET Framework. Given this, only the following will be considered:

* Bug fixes that are not reproducible on .NET Framework
* IL-neutral formatting changes done by Roslyn analyzers and automatic code fixers
* Performance fixes that bring .NET Core to performance parity with .NET Framework

All other changes will be tagged as 'future' and postponed until after .NET Core 3.0 GA in September.

**Please see the chart below for more details.**

| Category | Description | Accepted pre-GA |
| --- | --- | :---: |
| .NET Core code defect | .NET Core bugs that *are not* reproducible on .NET Framework will be accepted pre-GA. | :heavy_check_mark: |
| .NET Framework code defect | Fixes for .NET Core bugs that are **also** reproducible on .NET Framework will not be accepted. (If this was a recent regression in .NET Framework 4.8, or this blocks a core scenario, then we will likely consider bugs in this category). | :x: |
| .NET Core API change | An **enhancement** of an existing .NET Core API will not be accepted pre-GA. (We are also seeking to minimize deprecation and removal of APIs, unless they are unsupported by .NET core.)| :x: |
| New API (feature addition, public API) | New features and public APIs will not be considered until after .NET Core 3.0 GA. | :x: |
| Performance Optimization | We would like to achieve performance parity with .NET Framework. Fixes for performance regressions in .NET Core will be accepted, and other performance optimizations will be considered. | :heavy_check_mark: |
| Automated code formatting change | Automated IL-neutral changes with an associated Roslyn analyzer or editor config update will be accepted. Automated changes that result in differences to the IL will be individually considered. | :heavy_check_mark: |
| Manual code formatting change | Manual code formatting changes without an associated Roslyn analyzer or editor config update will not be accepted pre-GA. | :x: |

### General Guidelines

> 👉 **Remember\!** Your contributions may be incorporated into future versions of WPF\! Because of this, all pull requests will be subject to the same level of scrutiny for quality, coding standards, performance, globalization, accessibility, and compatibility as those of our internal contributors.

* **DO** create an issue before creating a pull request.
* **DO** create one pull request per Issue, and ensure that the Issue is linked in the pull request.
* **DO** follow our [coding and style guidelines](https://github.com/dotnet/corefx/blob/master/Documentation/coding-guidelines/coding-style.md), and keep code changes as small as possible.
* **DO** check for additional occurrences of the same problem in other parts of the codebase before submitting your PR.
* **DO** link the issue you are addressing in the pull request.
* **DO** write a good description for your pull request. More detail is better. Describe why the change is being made and why you have chosen a particular solution. Describe any manual testing you performed to validate your change.
* **DO** take an IL diff for code formatting changes and include IL-neutral or not IL-neutral in the PR.
* **DO NOT** submit a PR unless it is linked to an Issue marked triage approved. This enables us to have a discussion on the idea before anyone invests time in an implementation.
* **DO NOT** merge multiple changes into one PR unless they have the same root cause.
* **DO NOT** submit pure formatting/typo changes to code that has not been modified otherwise.



# Pull Request Process

### All pull requests must have an issue

1. If any guidance is required, please read the [contribution guidelines](contributing.md) and/or submit an issue requesting clarification.

2. Review the pre-GA acceptance table and open a new issue.

3. The issue will be triaged by someone from the WPF team.

4. If the issue meets the pre-GA acceptance criteria, the contributor will be assigned a WPF developer to help with the submission process. The issue will be assigned as a work item to the WPF developer.

5. The issue will be discussed with community members and WPF developers to understand the problem and review possible solutions.

6. When the a solution has been agreed upon, implement and **validate the change locally**.
> Please follow the testing requirements [Developer Guide](developer-guide.md).
> Verify your change works. Create and test the updated feature area locally with a WPF test application compiled against a version of the WPF Framework that contains your changes. See the [Developer Guide](developer-guide.md) for instructions on testing against local builds.
7. When the code is completed and has been verified locally, the contributor should submit a pull request that references the associated issue. Please complete the Contributor License Agreement if required.


8. GitHub checks will run against each new commit
- *GitHub Checks*

- License Check
- Style and Formatting
- Commit Message
- PR Build with Roslyn analyzers enabled

9. When the PR has been submitted and all GitHub checks pass (no build breaks or other issues) community members and WPF developers will review the code.

10. Additional internal testing may be performed by WPF developers if the change is determined to be high risk.

11. Community members and WPF developers need to review the change and sign-off on the pull request before the PR can be merged.

12. After the PR has been signed off, a WPF developer will manually squash and merge the PR.

#### At this point, the change is in master. Further internal testing will be performed. If there are no regressions, the change will be included in the next milestone release.

13. The internal developer regression test loop will be run against a build that includes the merged changes from the PR.

14. If a test fails, the squash-merge commit will be undone, and the WPF developer assigned to the issue will work with the submitter to root-cause and fix the regression. A new PR will need to be created by the submitter with the fixed code.

15. If the internal DRT test loop succeeds, the internal feature test loop will run. If this fails, the assigned WPF developer will work with the submitter to fix the regression and to submit a new PR with fixed code.

16. Finally, pre-milestone release, a full internal test pass is run. This test pass contains tens of thousands of tests across a large number of operating systems and machine configurations. If everything succeeds, the change (along with any others) will remain in master to be included in the next milestone release.

17. The repo is 'snapped' to a milestone release containing the PR.



### Would you like to contribute new features after September?

Please open an issue describing what feature area you would like to work on. We will begin assigning community owners and WPF developer peers some time in the future. Thanks!
13 changes: 3 additions & 10 deletions Documentation/contributing.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,30 +2,23 @@

The primary focus of .NET Core 3.0 release for WPF is to achieve parity with .NET Framework. Priority will be given to changes that align with that goal. See the [roadmap](../roadmap.md) to understand project goals.

We need the most help with the following types of changes:

* Test fixes, test improvements and new tests increasing code coverage.
* Bug fixes that specifically target partity between .NET Core and .NET Framework.
See the [acceptance criteria](acceptance_criteria.md) for types of issues that will be accepted before General Availability of .NET Core 3.0.

Please [file an issue](https://github.com/dotnet/wpf/issues) for any larger change you would like to propose.

See [Developer Guide](developer-guide.md) to learn how to develop changes for this repo.

This project follows the general [.NET Core Contribution Guidelines](https://github.com/dotnet/coreclr/blob/master/Documentation/project-docs/contributing.md). The contribution bar from the general contribution guidelines is copied below.

## Do not change files in the `Shared`, `WindowsBase`, `PresentationCore`, and `PresentationFramework` directories

The directories [`src/Microsoft.DotNet.Wpf/src/Shared`](https://github.com/dotnet/wpf/tree/master/src/Microsoft.DotNet.Wpf/src/Shared), [`src/Microsoft.DotNet.Wpf/src/WindowsBase`](https://github.com/dotnet/wpf/tree/master/src/Microsoft.DotNet.Wpf/src/WindowsBase), [`src/Microsoft.DotNet.Wpf/src/PresentationCore`](https://github.com/dotnet/wpf/tree/master/src/Microsoft.DotNet.Wpf/src/PresentationCore), [`src/Microsoft.DotNet.Wpf/src/PresentationFramework`](https://github.com/dotnet/wpf/tree/master/src/Microsoft.DotNet.Wpf/src/PresentationFramework) contains files that are shared between the public GitHub repo and the internal WPF repos. Changes to files in this directory will *not* be accepted since they also affect internal code. Once all the appropriate sources have been published to the public repo, this restriction will no longer be needed.

## Contribution "Bar"

Project maintainers will consider changes that improve the product or fix known bugs (please file issues to make bugs "known").

Maintainers will not merge changes that have narrowly-defined benefits due to compatibility risk or complexity added to the product. We may revert changes if they are found to be breaking.

Most .NET Core components are cross-platform and we appreciate contributions that either improve their feature set in a given environment or that add support for a new environment. We will typically not accept contributions that implement support for an OS-specific technolology on another operating system. For example, we do not intend to create an implementation of the Windows registry for Linux or an implementation of the macOS keychain for Windows. We also do not intend to accept contributions that provide cross-platform implementations for Windows Forms or WPF.
Most .NET Core components are cross-platform and we appreciate contributions that either improve their feature set in a given environment or that add support for a new environment. We will typically not accept contributions that implement support for an OS-specific technolology on another operating system. We also do not intend to accept contributions that provide cross-platform implementations for Windows Forms or WPF.

Contributions must also satisfy the other published guidelines defined in this document.
Contributions must also satisfy the [acceptance criteria](acceptance_criteria.md) to learn how to develop changes for this repo.as well as other published guidelines defined in this document.

## Code Formatting Improvements and Minor Enhancements

Expand Down
9 changes: 5 additions & 4 deletions Documentation/developer-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ of the repository when you do a full build using the `build.cmd` or `build.sh` s
You can run the copy-wpf.ps1 script again, except you can leave out the destination and be sure to pass in the
the `-testhost` parameter:
> eng\copy-wpf.ps1 -testhost
```cmd eng\copy-wpf.ps1 -testhost ```

You need to set environment variables so that your testhost installation is used when launching the application.
Once these are set, you should be able to launch the executable from the command line and then your assemblies
Expand All @@ -69,7 +70,7 @@ higher version number is chosen. Assuming our locally built binaries are newer t
installed, we can then simply reference those local binaries directly from the project file, like this:

*Note: you should build locally with the `-pack` param to ensure the binaries are put in the correct location.*
```
```xml
<PropertyGroup>
<!-- Change this value based on where your local repo is located -->
<WpfRepoRoot>d:\dev\src\dotnet\wpf</WpfRepoRoot>
Expand Down Expand Up @@ -106,7 +107,7 @@ Invoke-WebRequest https://dot.net/v1/dotnet-install.ps1 -OutFile $dotnet_install

This would install version `3.0.0-preview5-27619-18` of the `Microsoft.WindowsDesktop.App` shared runtime. You can pass `"Latest"` to get the latest version of the runtime. You can also use this script to install the runtimes as well as the SDK. If you know a particular SDK version and are curious to know what `Microsoft.WindowsDesktop.App` version is associated with it, there is a file called `Microsoft.NETCoreSdk.BundledVersions.props` contained inside the SDK folder. Inside that file, you will find an entry that looks like this:

```
```xml
<KnownFrameworkReference Include="Microsoft.WindowsDesktop.App"
TargetFramework="netcoreapp3.0"
RuntimeFrameworkName="Microsoft.WindowsDesktop.App"
Expand All @@ -124,7 +125,7 @@ In this example, the version of `Microsoft.WindowsDesktop.App` associated with t

#### Specifying which version of the runtime to use
If you can build directly from source, you can add this to your project file to pick up the version of the shared runtime you want to test:
```
```xml
<PropertyGroup>
<MicrosoftWindowsDesktopAppVersion>3.0.0-preview5-27619-18</MicrosoftWindowsDesktopAppVersion>
<PropertyGroup>
Expand All @@ -134,7 +135,7 @@ If you can build directly from source, you can add this to your project file to
```

If you don't have the ability to build from source, you can update the *.runtimeconfig.json file located next to the executable to pick up your version:
```
```json
{
"runtimeOptions": {
"tfm": "netcoreapp3.0",
Expand Down

0 comments on commit aa562e5

Please sign in to comment.