Skip to content

Commit

Permalink
Merge pull request #2078 from jnyrup/review_process
Browse files Browse the repository at this point in the history
Add issue templates and api review process
  • Loading branch information
jnyrup committed Jan 1, 2023
2 parents ca5c660 + c3a12ea commit a8b5257
Show file tree
Hide file tree
Showing 6 changed files with 264 additions and 50 deletions.
46 changes: 0 additions & 46 deletions .github/ISSUE_TEMPLATE.md

This file was deleted.

103 changes: 103 additions & 0 deletions .github/ISSUE_TEMPLATE/01_bug_report.yml
@@ -0,0 +1,103 @@
name: 🐞 Bug Report
description: Create a report to help us improve
labels: ["bug", "triage"]
body:
- type: markdown
attributes:
value: |
We welcome bug reports! Please see our [contribution guidelines](https://github.com/fluentassertions/fluentassertions/blob/develop/CONTRIBUTING.md#writing-a-good-bug-report) for more information on writing a good bug report.
**Before continuing, have you:**
* Tried upgrading to newest version of Fluent Assertions, to see if your issue has already been resolved and released?
* Checked existing open *and* closed [issues](https://github.com/fluentassertions/fluentassertions/issues?utf8=%E2%9C%93&q=is%3Aissue), to see if the issue has already been reported?
* Tried reproducing your problem in a new isolated project?
* Read the [documentation](https://fluentassertions.com/introduction)?
* Searched the two [test](https://github.com/fluentassertions/fluentassertions/tree/develop/Tests/FluentAssertions.Specs) [suites](https://github.com/fluentassertions/fluentassertions/tree/develop/Tests/FluentAssertions.Equivalency.Specs) if there is a test documenting the expected behavior?
* Considered if this is a general question and not a bug? For general questions please use [Stack Overflow](https://stackoverflow.com/questions/tagged/fluent-assertions?mixed=1).
- type: textarea
id: background
attributes:
label: Description
description: Please share a clear and concise description of the problem.
placeholder: Description
validations:
required: true
- type: textarea
id: repro-steps
attributes:
label: Reproduction Steps
description: |
Please include minimal steps to reproduce the problem if possible. E.g.: the smallest possible code snippet; or a small project, with steps to run it.
Always include text as text rather than screenshots so code can easily be copied and will show up in searches.
Stack Overflow has a great article about [how to create a minimal, reproducible example](https://stackoverflow.com/help/minimal-reproducible-example).
placeholder: Minimal Reproduction
value: |
```csharp
// Arrange
string input = "MyString";
// Act
char result = input[0];
// Assert
result.Should().Be('M');
```
validations:
required: true
- type: textarea
id: expected-behavior
attributes:
label: Expected behavior
description: |
Provide a description of the expected behavior.
placeholder: Expected behavior
validations:
required: true
- type: textarea
id: actual-behavior
attributes:
label: Actual behavior
description: |
Provide a description of the actual behavior observed. If applicable please include any error messages or exception stacktraces.
placeholder: Actual behavior
validations:
required: true
- type: textarea
id: regression
attributes:
label: Regression?
description: |
Did this work in a previous release of Fluent Assertions? If you can try a previous release to find out, that can help us narrow down the problem. If you don't know, that's OK.
placeholder: Regression?
validations:
required: false
- type: textarea
id: known-workarounds
attributes:
label: Known Workarounds
description: |
Please provide a description of any known workarounds.
placeholder: Known Workarounds
validations:
required: false
- type: textarea
id: configuration
attributes:
label: Configuration
description: |
Please provide more information on your .NET configuration:
* Which version of Fluent Assertions are you using?
* Which .NET runtime and version are you targeting? E.g. .NET framework 4.6.1 or .NET Core 2.1.
placeholder: Configuration
validations:
required: false
- type: textarea
id: other-info
attributes:
label: Other information
description: |
If you have an idea where the problem might lie, let us know that here. Please include any pointers to code, relevant changes, or related issues you know of.
placeholder: Other information
validations:
required: false
69 changes: 69 additions & 0 deletions .github/ISSUE_TEMPLATE/02_api_proposal.yml
@@ -0,0 +1,69 @@
name: 💡 API Suggestion
description: Propose a change to the public API surface
title: "[API Proposal]: "
labels: [api-suggestion]
body:
- type: markdown
attributes:
value: |
We welcome API proposals! We have a process to evaluate the value and shape of new API. There is an overview of our process [here](https://github.com/fluentassertions/fluentassertions/blob/develop/CONTRIBUTING.md#contributing-changes). This template will help us gather the information we need to start the review process.
- type: textarea
id: background
attributes:
label: Background and motivation
description: Please describe the purpose and value of the new API here.
placeholder: Purpose
validations:
required: true
- type: textarea
id: api-proposal
attributes:
label: API Proposal
description: |
Please provide the specific public API signature diff that you are proposing.
placeholder: API declaration (no method bodies)
value: |
```C#
public class EnumAssertions<TEnum, TAssertions>
{
public AndConstraint<TAssertions> BeDefined(string because = "", params object[] becauseArgs);
public AndConstraint<TAssertions> NotBeDefined(string because = "", params object[] becauseArgs);
}
```
validations:
required: true
- type: textarea
id: api-usage
attributes:
label: API Usage
description: |
Please provide code examples that highlight how the proposed API additions are meant to be consumed. This will help suggest whether the API has the right shape to be functional, performant and usable.
placeholder: API usage
value: |
```C#
var dayOfWeek = (DayOfWeek)1;
dayOfWeek.Should().BeDefined();
var invalidDayOfWeek = (DayOfWeek)999;
invalidDayOfWeek.Should().NotBeDefined();
```
validations:
required: true
- type: textarea
id: alternative-designs
attributes:
label: Alternative Designs
description: |
Please provide alternative designs. This might not be APIs; for example instead of providing new APIs an option might be to change the behavior of an existing API.
placeholder: Alternative designs
validations:
required: false
- type: textarea
id: risks
attributes:
label: Risks
description: |
Please mention any risks that to your knowledge the API proposal might entail, such as breaking changes, performance regressions, etc. If you are proposing a new overload of `Should()` include what type it currently resolves to for the type in question.
placeholder: Risks
validations:
required: false
14 changes: 14 additions & 0 deletions .github/ISSUE_TEMPLATE/config.yml
@@ -0,0 +1,14 @@
blank_issues_enabled: true
contact_links:
- name: 📚 Documentation
url: https://fluentassertions.com/introduction/
about: Read our comprehensive documentation.
- name: ⭐ Sponsor Fluent Assertions
url: https://github.com/sponsors/fluentassertions
about: Help the continued development.
- name: 💬 Ask on Stack Overflow
url: https://stackoverflow.com/questions/tagged/fluent-assertions
about: The best place for asking general purpose questions.
- name: 💬 Ask on Slack
url: https://fluentassertionsslack.herokuapp.com/
about: Get in touch with the whole community.
1 change: 1 addition & 0 deletions .github/PULL_REQUEST_TEMPLATE.md
@@ -1,5 +1,6 @@
## IMPORTANT

* [ ] If the PR touches the public API, the changes have been approved in a separate issue with the "api-approved" label.
* [ ] The code complies with the [Coding Guidelines for C#](https://www.csharpcodingguidelines.com/).
* [ ] The changes are covered by unit tests which follow the Arrange-Act-Assert syntax and the naming conventions such as is used [in these tests](../tree/develop/Tests/FluentAssertions.Equivalency.Specs/MemberMatchingSpecs.cs#L51-L430).
* [ ] If the PR adds a feature or fixes a bug, please update [the release notes](../tree/develop/docs/_pages/releases.md) with a functional description that explains what the change means to consumers of this library, which are published on the [website](https://fluentassertions.com/releases).
Expand Down
81 changes: 77 additions & 4 deletions CONTRIBUTING.md
@@ -1,10 +1,83 @@
# Contributing to Fluent Assertions

No open-source project is going to be successful without contributions. After we decided to move to Github, the involvement of the .NET community has increased significantly. However, contributing to this project involves a few steps that will seriously increase the chance we will accept it.
Few open-source projects are going to be successful without contributions.
Fluent Assertions is no exception and we are deeply grateful for all contributions no matter their size.
However, to improve that collaboration this document presents a few steps to smoothen the process.

* The [Pull Request](https://help.github.com/articles/using-pull-requests) is targeted at the `develop` branch.
* The code complies with the [Coding Guidelines for C#](https://csharpcodingguidelines.com/).
* The changes are covered by a new or existing set of unit tests which follow the Arrange-Act-Assert syntax such as is used [in this example](https://github.com/fluentassertions/fluentassertions/blob/daaf35b9b59b622c96d0c034e8972a020b2bee55/Tests/FluentAssertions.Shared.Specs/BasicEquivalencySpecs.cs#L33).
## Finding Existing Issues

Before filing a new issue, please search our [issues](https://github.com/fluentassertions/fluentassertions/issues) to check if it already exists.

If you do find an existing issue, please include your own feedback in the discussion.
Instead of posting "me too", upvote the issue with 👍, as this better helps us prioritize popular issues and avoids spamming people subscribing to the issue.

### Writing a Good Bug Report

Good bug reports make it easier for maintainers to verify and root cause the underlying problem.
The better a bug report, the faster the problem will be resolved.
Ideally, a bug report should contain the following information:

* A high-level description of the problem.
* A _minimal reproduction_, i.e. the smallest size of code/configuration required to reproduce the wrong behavior.
* A description of the _expected behavior_, contrasted with the _actual behavior_ observed.
* Information on the environment: nuget version, .NET version, etc.
* Additional information, e.g. is it a regression from previous versions? are there any known workarounds?

When ready to submit a bug report, please use the [Bug Report issue template](https://github.com/fluentassertions/fluentassertions/issues/new?labels=&template=01_bug_report.yml).

#### Why are Minimal Reproductions Important?

A reproduction lets maintainers verify the presence of a bug, and diagnose the issue using a debugger. A _minimal_ reproduction is the smallest possible console application demonstrating that bug. Minimal reproductions are generally preferable since they:

1. Focus debugging efforts on a simple code snippet,
2. Ensure that the problem is not caused by unrelated dependencies/configuration,
3. Avoid the need to share production codebases.

#### How to Create a Minimal Reproduction

The best way to create a minimal reproduction is gradually removing code and dependencies from a reproducing app, until the problem no longer occurs. A good minimal reproduction:

* Excludes all unnecessary types, methods, code blocks, source files, nuget dependencies and project configurations.
* Contains documentation or code comments illustrating expected vs actual behavior.

Stack Overflow has a great article about [how to create a minimal, reproducible example](https://stackoverflow.com/help/minimal-reproducible-example).

## Contributing Changes

Fluent Assertions exposes many extension knobs to write custom assertions or extend over existing assertions.
As such the core library very rarely takes extra dependencies to provide assertion on someones favorite library.
Instead we suggest that you create a dedicated library as we did with [FluentAssertions.Json](https://github.com/fluentassertions/fluentassertions.json).
See the [documentation](https://fluentassertions.com/extensibility/) for more information about extensibility.

In order for Fluent Assertions to provide a consistent experience across the library, we generally want to review every single API that is added, changed or deleted.
Changes to the API must be proposed, discussed and approved with the `api-approved` label in a separate issue before opening a PR.
Sometimes the implementation leads to new knowledge such that the approved API must be reevaluated.
If you're unsure about whether a change fits the library we suggest you open an issue first to avoid wasting your time if the changes does not fit the project.

Also we balance whether proposed features are too niche or complex to pull their weight.
A feature proposal so to speak starts at [-100 points](https://web.archive.org/web/20200112182339/https://blogs.msdn.microsoft.com/ericgu/2004/01/12/minus-100-points/) and needs to prove its worth.
Remember that a rejection of an API approval is not necessarily a rejection of your idea, but merely a rejection of including it in the core library.

When ready to submit a proposal, please use the [API Suggestion issue template](https://github.com/fluentassertions/fluentassertions/issues/new?labels=api-suggestion&template=02_api_proposal.yml&title=%5BAPI+Proposal%5D%3A+).

Contributions must also satisfy the other published guidelines defined in this document.

### DOs and DON'Ts

Please do:

* Target the [Pull Request](https://help.github.com/articles/using-pull-requests) at the `develop` branch.
* Follow the style presented in the [Coding Guidelines for C#](https://csharpcodingguidelines.com/).
* Align with the [Design Principles](https://github.com/fluentassertions/fluentassertions/issues/1340)
* Ensure that changes are covered by a new or existing set of unit tests which follow the Arrange-Act-Assert syntax such as is used [in this example](https://github.com/fluentassertions/fluentassertions/blob/daaf35b9b59b622c96d0c034e8972a020b2bee55/Tests/FluentAssertions.Shared.Specs/BasicEquivalencySpecs.cs#L33).
* Also the code coverage reported by the coveralls must be non-decreasing unless accepted by the authors.
* If the contribution adds a feature or fixes a bug, please update the [**release notes**](https://github.com/fluentassertions/fluentassertions/blob/develop/docs/_pages/releases.md), which is published on the [website](https://fluentassertions.com/releases).
* If the contribution changes the public API, the changes needs to be included by running [`AcceptApiChanges.ps1`](https://github.com/fluentassertions/fluentassertions/tree/develop/AcceptApiChanges.ps1)/[`AcceptApiChanges.sh`](https://github.com/fluentassertions/fluentassertions/tree/develop/AcceptApiChanges.sh) or using Rider's [Verify Support](https://plugins.jetbrains.com/plugin/17240-verify-support) plug-in.
* If the contribution affects the documentation, please update the [**documentation**](https://github.com/fluentassertions/fluentassertions/tree/develop/docs/_pages), under the appropriate file (i.e. [strings.md](https://github.com/fluentassertions/fluentassertions/blob/develop/docs/_pages/strings.md) for changes to string assertions), which is published on the [website](https://fluentassertions.com/introduction).

Please do not:

* **DON'T** surprise us with big pull requests. Instead, file an issue and start
a discussion so we can agree on a direction before you invest a large amount
of time. This includes _any_ change to the public API.
* Approved API changes are labeled with `api-approved`.

0 comments on commit a8b5257

Please sign in to comment.