diff --git a/docs/content/contributing/submitting-pull-requests.md b/docs/content/contributing/submitting-pull-requests.md index 835482189..ec26125f9 100644 --- a/docs/content/contributing/submitting-pull-requests.md +++ b/docs/content/contributing/submitting-pull-requests.md @@ -3,12 +3,228 @@ title: "Traefik Pull Requests Documentation" description: "Looking to contribute to Traefik Proxy? This guide will show you the guidelines for submitting a PR in our contributors guide repository." --- -# Submitting Pull Requests +# Before You Submit a Pull Request -A Quick Guide for Efficient Contributions -{: .subtitle } +This guide is for contributors who already have a pull request to submit. +If you are looking for information on setting up your developer environment +and creating code to contribute to Traefik Proxy or related projects, +see the [development guide](https://docs.traefik.io/contributing/building-testing/). -So you've decided to improve Traefik? -Thank You! +Looking for a way to contribute to Traefik Proxy? +Check out this list of [Priority Issues](https://github.com/traefik/traefik/labels/contributor%2Fwanted), +the [Good First Issue](https://github.com/traefik/traefik/labels/contributor%2Fgood-first-issue) list, +or the list of [confirmed bugs](https://github.com/traefik/traefik/labels/kind%2Fbug%2Fconfirmed) waiting to be remedied. -Please review the [guidelines on creating PRs](https://github.com/traefik/contributors-guide/blob/master/pr_guidelines.md) for Traefik in our [contributors guide repository](https://github.com/traefik/contributors-guide). +## How We Prioritize + +We wish we could review every pull request right away. +Unfortunately, our team has to prioritize pull requests (PRs) for review +(but we are welcoming new [maintainers](https://github.com/traefik/traefik/blob/master/docs/content/contributing/maintainers-guidelines.md) to speed this up, +so if you are interested, check it out and apply). + +The PRs we are able to handle fastest are: + +* Documentation updates. +* Bug fixes. +* Enhancements and Features with a `contributor/wanted` tag. + +PRs that take more time to address include: + +* Enhancements or Features without the `contributor/wanted` tag. + +If you have an idea for an enhancement or feature that you would like to build, +[create an issue](https://github.com/traefik/traefik/issues/new/choose) for it first +and tell us you are interested in writing the PR. +If an issue already exists, definitely comment on it to tell us you are interested in creating a PR. + +This will allow us to communicate directly and let you know if it is something we would accept. +It also allows us to make sure you have all the information you need during the design phase +so that it can be reviewed and merged quickly. + +If you have questions about the Triage process, +[read more here](https://github.com/traefik/contributors-guide/blob/master/issue_triage.md). + +## The Pull Request Submit Process + +Merging a PR requires the following steps to be completed before it is merged automatically. + +* Make sure your pull request adheres to our best practices. These include: + * [Following project conventions](https://github.com/traefik/traefik/blob/master/docs/content/contributing/maintainers-guidelines.md); including using the PR Template. + * Make small pull requests. + * Solve only one problem at a time. + * Comment thoroughly. + * Do not open the PR from an organization repository. + * Keep "allows edit from maintainer" checked. + * Use semantic line breaks for documentation. +* Pass the validation check. +* Pass all tests. +* Receive 3 approving reviews maintainers. + +## Pull Request Review Cycle + +You can read about our Triage Process [here](https://github.com/traefik/contributors-guide/blob/master/issue_triage.md), +but in short, it looks like this: + +* We triage every new PR or comment before entering it into the review process. + * We ensure that all prerequisites for review have been met. + * We check to make sure the use case meets our needs. + * We assign reviewers. +* Design Review. + * This takes longer than other parts of the process. + * We review that there are no obvious conflicts with our codebase. +* Code Review. + * We review the code in-depth and run tests. + * We may ask for changes here. + * During code review, we ask that you be reasonably responsive, + if a PR languishes in code review it is at risk of rejection, + or we may take ownership of the PR and the contributor will become a co-author. +* Merge. + * Success! + +!!! note + + Occasionally, we may freeze our codebase when working towards a specific feature or goal that could impact other development. + During this time, your pull request could remain unmerged while the release work is completed. + +## Run Local Verifications + +You must run these local verifications before you submit your pull request to predict the pass or failure of continuous integration. +Your PR will not be reviewed until these are green on the CI. + +* `make validate` +* `make pull-images` +* `make test` + +## The Testing and Merge Workflow + +Pull Requests are managed by the bot [Myrmica Lobicornis](https://github.com/traefik/lobicornis). +This bot is responsible for verifying GitHub Checks (CI, Tests, etc), mergability, and minimum reviews. +In addition, it rebases or merges with the base PR branch if needed. +It performs several other housekeeping items +and you can read more about those on the [README](https://github.com/traefik/lobicornis) for Lobicornis. + +The maintainer giving the final LGTM must add the `status/3-needs-merge` label to trigger the merge bot. + +By default, a squash-rebase merge will be carried out. + +The status `status/4-merge-in-progress` is only used by the bot. + +If the bot is not able to perform the merge, the label `bot/need-human-merge` is added. +In such a situation, solve the conflicts/CI/... and then remove the label `bot/need-human-merge`. + +To prevent the bot from automatically merging a PR, add the label `bot/no-merge`. + +The label `bot/light-review` decreases the number of required LGTM from 3 to 1. + +This label can be used when: + +* Updating a dependency. +* Merging branches back into the next version branch. +* Submitting minor documentation changes. +* Submitting changelog PRs. + +## Why Was My Pull Request Closed? + +Traefik Proxy is made by the community for the community, +as such the goal is to engage the community to make Traefik the best reverse proxy available. +Part of this goal is maintaining a lean codebase and ensuring code velocity. +unfortunately, this means that sometimes we will not be able to merge a pull request. + +Because we respect the work you did, you will always be told why we are closing your pull request. +If you do not agree with our decision, do not worry; closed pull requests are easy to recreate, +and little work is lost by closing a pull request that subsequently needs to be reopened. + +Your pull request might be closed if: + +* Your PR's design conflicts with our existing codebase in such a way that Merging is not an option + and the work needed to make your pull request usable is too high. + * To prevent this, make sure you created an issue first + and think about including Traefik Proxy maintainers in your design phase to minimize conflicts. +* Your PR is for an enhancement or feature that we will not use. + * Please remember to create an issue for any pull request **before** you create a PR + to ensure that your goal is something we can merge and that you have any design insight you might need from the team. +* Your PR has been waiting for feedback from the contributor for over 90 days. + +## Why is My Pull Request Not Getting Reviewed + +A few factors affect how long your pull request might wait for review. + +We must prioritize which PRs we focus on. +Our first priority is PRs we have identified as having high community engagement and broad applicability. +We put our top priorities on our roadmap and you can identify them by the `contributor/wanted` tag. +These PRs will enter our review process the fastest. + +Our second priority is bug fixes. +Especially for bugs that have already been tagged with `bug/confirmed`. +These reviews enter the process quickly. + +If your PR does not meet the criteria above, +it will take longer for us to review as any PRs that do meet the criteria above will be prioritized. + +Additionally, during the last few weeks of a milestone, we stop reviewing PRs to reduce churn and stabilize. +We will resume after the release. + +The second major reason that we deprioritize your PR is that you are not following best practices. + +The most common failures to follow best practices are: + +* You did not create an issue for the PR you wish to make. + If you do not create an issue before submitting your PR, + we will not be able to answer any design questions and let you know how likely your PR is to be merged. +* You created pull requests that are too large to review. + * Break your pull requests up. + If you can extract whole ideas from your pull request and send those as pull requests of their own, + you should do that instead. + It is better to have many pull requests addressing one thing than one pull request addressing many things. + * Traefik Proxy is a fast-moving codebase — lock in your changes ASAP with your small pull request, + and make merges be someone else's problem. + We want every pull request to be useful on its own, + so use your best judgment on what should be a pull request vs. a commit. +* You did not comment well. + * Comment everything. + +Please remember that we are working internationally, cross-culturally, and with different use-cases. +Your reviewer will not intuitively understand the problem the same way you do or solve it the same way you would. +This is why every change you make must be explained and your strategy for coding must also be explained. + +* Your tests were inadequate or absent. + * If you do not know how to test your PR, please ask! + We will be happy to help you or suggest appropriate test cases. + +If you have already followed the best practices and your PR still has not received a response, +here are some things you can do to move the process along: + +* If you have fixed all the issues from a review, + remember to re-request a review (using the designated button) to let your reviewer know that you are ready. + You can choose to comment with the changes you made. +* Ping `@tfny` if you have not been assigned to a reviewer. + +For more information on best practices, try these links: + +* [How to Write a Git Commit Message - Chris Beams](https://chris.beams.io/posts/git-commit/) +* [Distributed Git - Contributing to a Project (Commit Guidelines)](https://git-scm.com/book/en/v2/Distributed-Git-Contributing-to-a-Project) +* [What’s with the 50/72 rule? - Preslav Rachev](https://preslav.me/2015/02/21/what-s-with-the-50-72-rule/) +* [A Note About Git Commit Messages - Tim Pope](https://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html) + +## It's OK to Push Back + +Sometimes reviewers make mistakes. +It is OK to push back on changes your reviewer requested. +If you have a good reason for doing something a certain way, you are absolutely allowed to debate the merits of a requested change. +Both the reviewer and reviewee should strive to discuss these issues in a polite and respectful manner. + +You might be overruled, but you might also prevail. +We are pretty reasonable people. + +Another phenomenon of open-source projects (where anyone can comment on any issue) is the dog-pile - +your pull request gets so many comments from so many people it becomes hard to follow. +In this situation, you can ask the primary reviewer (assignee) whether they want you to fork a new pull request +to clear out all the comments. +You do not have to fix every issue raised by every person who feels like commenting, +but you should answer reasonable comments with an explanation. + +## Common Sense and Courtesy + +No document can take the place of common sense and good taste. +Use your best judgment, while you put a bit of thought into how your work can be made easier to review. +If you do these things your pull requests will get merged with less friction.