Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,46 @@ To prevent closure, simply add comments, push commits, or respond to feedback.

Security issues and bugs should be reported privately, via email, to the Microsoft Security Response Center (MSRC) [secure@microsoft.com](mailto:secure@microsoft.com). You should receive a response within 24 hours. If for some reason you do not, please follow up via email to ensure we received your original message. Further information, including the MSRC PGP key, can be found in the [MSRC FAQ](https://www.microsoft.com/en-us/msrc/faqs-report-an-issue?rtc=1&oneroute=true).

## Submitting Pull Requests

- **New features from community PRs must be driven by creating a GitHub issue first.** Discuss the proposal in the issue before starting implementation. This helps avoid wasted effort and ensures alignment with project goals.
- **Community contributions must not derail the project roadmap.** We prioritize features and fixes according to our published milestones. PRs that conflict with or distract from active roadmap items may be deferred.
- **Our maintainers reserve the right to reject PRs** that do not meet the required criteria to qualify for review. This includes PRs that:
- Lack a corresponding approved issue (i.e., an issue that has been reviewed, acknowledged, and agreed upon by maintainers — typically indicated by the **`PM Approved`** field being set to **`Approved`** in the GitHub Project board)
- Introduce breaking changes without prior discussion
Comment thread
Copilot marked this conversation as resolved.
- Do not follow the project's coding standards and conventions
- Are missing adequate test coverage
- Conflict with work already in progress by the team
- **Bug fixes and small improvements** are generally welcome without a prior issue, but a linked issue helps us triage and prioritize your contribution.

### What Makes a Great Contribution

1. **Start with an issue** — File a [feature request](https://github.com/dotnet/SqlClient/issues/new?template=feature_request.md) or [bug report](https://github.com/dotnet/SqlClient/issues/new?template=bug-report.md) and wait for maintainer feedback
2. **Follow the conventions** — See [coding guidelines](policy/coding-style.md)
3. **Include tests** — Both unit tests and integration tests where applicable
4. **Keep scope focused** — One feature or fix per PR
5. **Update documentation** — For any public API changes

### Contribution Workflow

See [contributing-workflow.md](contributing-workflow.md) for details on how PRs are tracked through our review process.

## Feedback

The best way to give feedback is to create issues in the [dotnet/SqlClient](https://github.com/dotnet/SqlClient) repo.

Please give us feedback that will provide insight on the following:

- Existing features that are missing some capability or otherwise don't work well enough.
- Missing features that should be added to the product.
- Design choices for a feature that is currently in-progress.

Some important caveats:

- It is best to give design feedback quickly for improvements that are in development. We're unlikely to hold a feature from a release on late feedback.
- We are most likely to include improvements that either have a positive impact on a broad scenario or have very significant positive impact on a niche scenario. This means that we are unlikely to prioritize modest improvements to niche scenarios.
- Compatibility will almost always be given a higher priority than improvements.

## Contribution Standards

Project maintainers will merge changes that improve the product significantly and broadly and that align with the [Microsoft.Data.SqlClient roadmap](roadmap.md).
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ When targeting .NET on Windows, a package reference to [Microsoft.Data.SqlClient
| Coding Style | [coding-style.md](/policy/coding-style.md) |
| Contributing | [CONTRIBUTING.md](CONTRIBUTING.md) |
| Copyright Information | [COPYRIGHT.md](COPYRIGHT.md) |
| Roadmap | [roadmap.md](roadmap.md) |
| Review Process | [review-process.md](/policy/review-process.md) |
| Support Policy | [SUPPORT.md](SUPPORT.md) |

Expand Down
93 changes: 90 additions & 3 deletions contributing-workflow.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ You can contribute to Microsoft.Data.SqlClient with issues and PRs. Simply filin
We use and recommend the following workflow:

1. **Create an issue for your work.**
- You can skip this step for trivial changes.
- **For all significant changes you intend to make in this repository, an issue is required.** This gives maintainers a chance to review the proposal before implementation begins and prevents wasted effort. Bug fixes and small improvements are generally welcome without a prior issue, but a linked issue helps us triage and prioritize your contribution.
- Reuse an existing issue on the topic, if there is one.
- Get agreement from the team and the community that your proposed change is a good one.
- Suggest [Labels](CONTRIBUTING.md#using-labels) to add for your issue.
Expand Down Expand Up @@ -39,11 +39,12 @@ We use and recommend the following workflow:
- It is OK to create your PR as Draft on the upstream repo before the implementation is done.
- This can be useful if you'd like to start the feedback process concurrent with your implementation.
- State that this is the case in the initial PR comment.
- **Keep your PR in Draft until CI validation passes.** The PR pipeline runs without maintainer intervention, so contributors should ensure all checks are green before marking the PR as ready for review. PRs appearing ready for review with failing checks will not be picked up by reviewers.

### Commit Guidelines

- It is OK for your PR to include a large number of commits.
- Once your change is accepted, you will be asked to squash your commits into one or some appropriately small number of commits before your PR is merged.
- PRs are merged via the default "Squash and Merge" strategy (see [Merging Pull Requests](#merging-pull-requests)), so your commit history is automatically collapsed into a single commit at merge time.

## PR - CI Process

Expand All @@ -63,13 +64,98 @@ If the CI build fails for any reason, the PR issue will be updated with a link t

- **Build failures**: Check the CI logs for specific error messages.
- **Test failures**: Ensure all tests pass locally before pushing.
- **Merge conflicts**: Rebase your branch against the latest main branch.
- **Merge conflicts**:
- *Before review has started*: Rebase your branch against the latest `main` branch to keep history clean.
- *After a reviewer has started reviewing*: Do **not** rebase. Rebasing rewrites commit history and makes it impossible for reviewers to tell which commits they have already reviewed, forcing them to start over. Instead, resolve conflicts by merging the latest `main` into your branch (`git merge main`) so that previously reviewed commits remain intact and reviewers can pick up where they left off.
Comment thread
paulmedynski marked this conversation as resolved.

### Getting Help

- Comment on your PR if you're stuck.
- Reach out in [GitHub Discussions](https://github.com/dotnet/SqlClient/discussions) for broader questions.

## Tracking PRs in the GitHub Project

The [SqlClient Board](https://github.com/orgs/dotnet/projects/588) (dotnet org project #588) is the team's triage board for tracking issues and PRs. When a PR is linked to an issue, its progress is tracked through the project's fields described below.

### Project Fields

| Field | Type | Values | Purpose |
|-------|------|--------|---------|
| **Status** | Single select | `To triage`, `Waiting for customer`, `In progress`, `In review`, `In validation`, `Done` | Tracks the current workflow stage of the item |
| **Priority** | Single select | `P0`, `P1`, `P2`, `P3` | Indicates urgency and scheduling priority |
| **Size** | Single select | `XS`, `S`, `M`, `L`, `XL` | Estimates effort/complexity of the work |
| **API Impact** | Single select | `Breaking Change`, `New API`, `None` | Flags whether the change affects public API surface |
| **PM Approved** | Single select | `N/A`, `Pending`, `Approved` | Tracks product manager sign-off for API or behavioral changes |
| **Assignees** | Field | *(GitHub users)* | Who is responsible for the work |
| **Labels** | Field | *(GitHub labels)* | Categorization and area tags |
| **Reviewers** | Field | *(GitHub users)* | Assigned code reviewers |
| **Milestone** | Field | *(GitHub milestones)* | Target release version |
| **Linked pull requests** | Field | *(PR references)* | PRs associated with the issue |
| **Parent issue** | Field | *(issue reference)* | Epic or parent tracking issue |
| **Sub-issues progress** | Field | *(auto-calculated)* | Completion progress of child issues |
| **Comment** | Text | *(free text)* | Additional context or notes |
| **AB-Link** | Text | *(URL)* | Link to Azure Boards work item (if applicable) |

### Status Stages for PRs

PRs (and their linked issues) move through the **Status** field to communicate reviewability and progress:

| Status | What it means for a PR |
|--------|------------------------|
| **To triage** | PR just opened or linked issue is awaiting initial assessment. Maintainers will review scope, assign reviewers, and set priority. |
| **In progress** | Author is actively developing the change. PR may be in Draft state. |
| **In review** | PR is ready for code review. Reviewers are assigned and the author considers the implementation complete. |
| **Waiting for customer** | Review feedback has been given; the PR author needs to respond or push changes. When a reviewer requests changes, they will also apply the **`Author attention needed`** label so the PR is easy to surface in queries and dashboards. Once the author addresses the feedback, they should remove the `Author attention needed` label if permissions allow or comment `/ready` on the PR to remove the label `Author attention needed` and re-engage the reviewers to move the PR back to `In review`. |
| **In validation** | PR is approved and being validated (CI, manual testing, integration checks) before merge. |
| **Done** | PR has been merged and the associated work is complete. |

### Status Transitions

When a reviewer leaves feedback that needs author action, they move the PR to `Waiting for customer` **and** apply the `Author attention needed` label. The author removes the label (and updates Status back to `In review`/`In progress`) once the feedback is addressed.

```
+--------------+ +---------------+ +-------------------------+ +-----------------+ +--------+
| To triage |------>| In progress |------>| In review |------>| In validation |------>| Done |
+--------------+ +---------------+ +------------+------------+ +-----------------+ +--------+
^ | ^
| | changes | author addresses
| | requested | feedback (minor) +
| | + label | removes label
| v |
| +------------------------+
| | Waiting for customer |
| | + Author attention |
| | needed (label) |
| +------------------------+
| |
| | major rework needed
+----------------------+
```

### Additional Field Usage

- **Priority**: `P0` items are critical fixes that should be reviewed and merged urgently. `P1` items are high priority for the current milestone. `P2`/`P3` items are scheduled as capacity allows.
- **Size**: Helps reviewers estimate review effort. `XS`/`S` PRs should get faster turnaround. `L`/`XL` PRs may need multiple reviewers or phased review.
- **API Impact**: PRs marked `Breaking Change` or `New API` require **PM Approved = Approved** before merge. `ref/` project updates must accompany these PRs.
- **PM Approved**: Set to `Pending` when a PR introduces API changes. Must reach `Approved` before the PR can be merged. `N/A` for internal or non-API changes.

### Guidelines for Contributors

> **Note:** Setting labels and project board fields (Status, Priority, etc.) requires maintainer or triage access. External contributors cannot modify these directly. Instead, suggest labels in your PR description and the team will apply them. If review feedback asks you to "remove the label" or "update Status", a maintainer will handle it if you don't have access—just let the team know in a comment.

1. **Link your PR to an issue** — This ensures the PR appears on the project board and is tracked through the workflow.
2. **Keep Status current** — If you're the author, move your linked issue to `In review` when your PR is ready for feedback.
3. **Respond promptly** — When status is `Waiting for customer` and the **`Author attention needed`** label is applied, the team is blocked on your response. Address the feedback, push the updates, **remove the `Author attention needed` label**, and move Status back to `In review` (or `In progress` for major rework) so reviewers know the PR is ready for another pass. If you are a community contributor, you may post a comment `/ready` on the PR to automate removing the label and re-engaging reviewers on the PR.
4. **Flag API changes early** — Set **API Impact** appropriately so PM review can happen in parallel with code review.
5. **Don't skip validation** — Even after approval, the PR stays in `In validation` until CI passes and any manual verification is complete.
Comment thread
paulmedynski marked this conversation as resolved.

### Guidelines for Reviewers

1. **Signal that author action is required** — When your review requests changes (whether via formal "Request changes" or a comment that requires author follow-up), apply the **`Author attention needed`** label to the PR and move its linked issue to `Waiting for customer`. This pair (label + status) is what the team uses to find PRs that are blocked on the author versus those still awaiting review.
2. **Don't remove the label yourself** — Leave the `Author attention needed` label in place until the author pushes updates addressing the feedback; the author is responsible for removing it when they hand the PR back for review.
3. **Re-review promptly** — Once the author removes the label, the PR status should be updated either by Author (if they have permissions) or the reviewer to `In review`, so it doesn't stall and is picked up for review on time.
4. **Only reviewers resolve their own feedback threads** — A review comment thread should only be resolved by the reviewer who created it, once they are satisfied the feedback has been addressed. Authors should not resolve reviewer threads themselves.

## Stale PR Management

The SqlClient repository uses automated workflows to manage inactive pull requests and maintain repository hygiene.
Expand Down Expand Up @@ -111,6 +197,7 @@ One or more Microsoft team members will review every PR prior to merge. They wil
- Address all review comments before the PR can be merged.
- Feel free to ask questions if feedback is unclear.
- Push additional commits to address feedback (these will be squashed later).
- When a reviewer leaves feedback that needs your action, they will set Status to `Waiting for customer` and apply the **`Author attention needed`** label. After you push updates, **remove the `Author attention needed` label** or post a comment `/ready` to do so, so the reviewers know the PR is ready for another pass.

## Merging Pull Requests

Expand Down
Loading
Loading