From 230a80d34f9565d642812308c4c5477449b2569c Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Thu, 5 Oct 2023 17:12:49 +1300 Subject: [PATCH] DOC Update contributing code page --- en/05_Contributing/01_Code.md | 375 +++++++++++++++++++--------------- 1 file changed, 207 insertions(+), 168 deletions(-) diff --git a/en/05_Contributing/01_Code.md b/en/05_Contributing/01_Code.md index df55aa90f..d3676b3d9 100644 --- a/en/05_Contributing/01_Code.md +++ b/en/05_Contributing/01_Code.md @@ -6,245 +6,284 @@ icon: code # Contributing Code - Submitting Bugfixes and Enhancements -Silverstripe CMS will never be finished, and we need your help to keep making it better. If you're a developer a great way to get involved is to contribute patches to our modules and core codebase, fixing bugs or adding features. +The Silverstripe CMS core and supported modules are hosted on [GitHub](https://github.com) - mostly in [github.com/silverstripe](https://github.com/silverstripe/). To contribute code, you will need to [create a GitHub account](https://docs.github.com/en/get-started/onboarding/getting-started-with-your-github-account). -The Silverstripe CMS core modules (`framework` and `cms`), as well as some of the more popular modules are in -git version control. Silverstripe CMS hosts its modules on [github.com/silverstripe](https://github.com/silverstripe/). After [installing git](https://help.github.com/articles/set-up-git/) and creating a [free github.com account](https://github.com/join/), you can "fork" a module, -which creates a copy that you can commit to (see github's [guide to "forking"](https://help.github.com/articles/fork-a-repo/)). - -For other modules, our [add-ons site](https://addons.silverstripe.org/add-ons/) lists the repository locations, typically using the version control system like "git". - -If you are modifying CSS or JavaScript files in core modules, you'll need to regenerate some files. -Please check out our [client-side build tooling](build_tooling) guide for details. +This documentation assumes you are fairly confident with git and GitHub. If that isn't the case, you may want to read some guides for [GitHub](https://docs.github.com/en/get-started/quickstart), [git](https://docs.github.com/en/get-started/using-git), and [pull requests](https://docs.github.com/en/pull-requests). [hint] -Note: By supplying code to the Silverstripe CMS core team in patches, tickets and pull requests, you agree to assign copyright of that code to Silverstripe Limited, on the condition that Silverstripe Limited releases that code under the BSD license. +Note: By supplying code to the Silverstripe CMS core team in issues and pull requests, you agree to assign copyright of that code to Silverstripe Limited, on the condition that Silverstripe Limited releases that code under the BSD license. -We ask for this so that the ownership in the license is clear and unambiguous, and so that community involvement doesn't stop us from being able to continue supporting these projects. By releasing this code under a permissive license, this copyright assignment won't prevent you from using the code in any way you see fit. +We ask for this so that the ownership in the license is clear and unambiguous, and so that community involvement doesn't stop us from being able to continue supporting these projects. By releasing this code under a permissive license, this copyright assignment won't prevent you from using the code in any way you see fit. [/hint] -## Step-by-step: From forking to sending the pull request +## Before you start working {#before-you-start} -[notice] -**Note:** Please adjust the commands below to the version of Silverstripe CMS that you're targeting. -[/notice] +There are a few things that you should do before you start working on a fix: -1. Create a [fork](https://help.github.com/articles/about-forks/) of the module you want to contribute to (listed on [github.com/silverstripe/](https://github.com/silverstripe/)). +### Consider if your change should be its own module -1. Install the project through composer. The process is described in detail in "[Installation through Composer](../getting_started/composer#contributing)". +Not every feature belongs in the core modules - consider whether the change you want to make belongs in core or whether it would be more appropriate for you to [create a new module](/developer_guides/extending/modules/#create). - composer create-project --keep-vcs silverstripe/installer ./your-website-folder 4.0.x-dev +### Check for an existing pull request -1. Add a new "upstream" remote to the module you want to contribute to (e.g. `cms`). -This allows you to track the original repository for changes, and rebase/merge your fork as required. -Use your Github user name for which you created the fork in Step 1. +Check to see if someone else has already submitted a pull request for this change by searching on GitHub. If they have, consider collaborating with them by reviewing their PR. - cd framework - git remote rename origin upstream - git branch --set-upstream-to upstream - git remote add -f origin git://github.com//silverstripe-framework.git +### Make or find a GitHub issue -1. [Branch for new issue and develop on issue branch](code#branch-for-new-issue-and-develop-on-issue-branch) +Whether you're fixing a bug, updating documentation, making an ehancement for an existing feature, or even a brand new feature, you must link your pull request to a GitHub issue. - # verify current branch 'base' then branch and switch - cd framework - git status - git checkout -b +If there's an existing GitHub issue, there may already be some discussion there about the preferred approach. Make sure you read through the comments. -1. As time passes, the upstream repository accumulates new commits. Keep your working copy's branch and issue branch up to date by periodically running a `composer update`. -As a first step, make sure you have committed all your work, then temporarily switch over to the `master` branch while updating. -Alternatively, you can use [composer "repositories"](https://getcomposer.org/doc/05-repositories.md#vcs), -but we've found that dramatically slows down any updates. You may need to [resolve conflicts](https://help.github.com/articles/resolving-merge-conflicts-after-a-git-rebase/). +If there _isn't_ an existing issue, you should create one. Make sure you mention in your issue that you intend to make a pull request to implement the change (especially if this is for a new feature). - (cd framework && git checkout master) - composer update - (cd framework && git checkout ) - (cd framework && git rebase upstream/master) +If you are planning to develop an extensive feature or fix a bug that could have wide-reaching effects, try to get some discussion in your issue before you do much coding. Make it clear in the issue that you want to discuss it before working on it, and consider discussing the problem in one of the [community channels](https://www.silverstripe.org/community/) (and summarise the discussion in the issue afterward). -1. When development is complete, run another update, and consider [squashing your commits](https://help.github.com/articles/using-git-rebase-on-the-command-line/) +Refer to [Contributing Issues](./issues_and_bugs/) for more information about finding and creating GitHub issues. -1. Push your branch to your GitHub fork +## Step-by-step: how to contribute code {#step-by-step} - cd framework - git push origin +[notice] +The examples below assume you are making a change that applies to the `4.13` branch. -1. Issue pull request on GitHub. Visit your forked repository on gitHub.com and click the "Create Pull Request" button next to the new branch. +Please adjust the commands as appropriate for the version of Silverstripe CMS that you're targeting. See [picking the right version](#picking-the-right-version). +[/notice] -Please read [collaborating with pull requests](https://help.github.com/categories/collaborating-with-issues-and-pull-requests/) on github.com -for more details. +### Editing files directly on GitHub.com -The core team is then responsible for reviewing patches and deciding if they will make it into core. If -there are any problems they will follow up with you, so please ensure they have a way to contact you! +If you see a typo or another small fix that needs to be made, and you don't have an installation set up for contributions, you can edit files directly in the github.com web interface. Every file view on GitHub has an "edit this file" link. -### Picking the right version +After you have edited the file, GitHub will offer to create a pull request for you. This pull request will be reviewed along with other pull requests. -The Silverstripe CMS project follows the [Semantic Versioning](http://semver.org) (SemVer) specification for releases. -It clarifies what to expect from different releases, and also guides you in choosing the right branch to base your pull request on. +Make sure you read the [picking the right version](#picking-the-right-version), [create the pull request](#create-the-pr), and [recieve and respond to feedback](#recieve-feedback) sections below. -If you are unsure what branch your pull request should go to, consider asking in the GitHub issue that you address with your patch, or -simply choose the "default branch" of the repository where you want to contribute to. That would usually target the next minor release of the module. +### Step 1: Picking the right version {#picking-the-right-version} -If you are changing existing APIs, introducing new APIs or major new features, -please review our guidance on [supported versions](release_process#supported-versions). -You will need to choose the branch for your pull request accordingly. +The Silverstripe CMS project follows [Semantic Versioning](https://semver.org), which clarifies what to expect from different releases and also guides you in choosing the right branch to base your pull request on. -As we follow SemVer, we name the branches in repositories accordingly -(using BNF rules defined by [semver.org](https://semver.org/#backusnaur-form-grammar-for-valid-semver-versions)): - - `` branches contain major versions and all changes for yet unreleased minor versions - - ` "." ` branches contain released minor versions and all changes for yet to be released patch versions +As we follow semantic versioning, we name the branches in repositories accordingly: +- `` (e.g. `4`) branches contain all changes for upcoming major or minor releases. These are called "major release branches" or "minor release branches", depending on whether they represent the next major release or the next minor release. +- `.` (e.g. `4.13`) branches contain all changes for upcoming patch releases. These are called "patch release branches". -Silverstripe CMS public APIs explicitly include: - - namespaces, classes, interfaces and traits - - public and protected scope (including methods, properties and constants) - - global functions, variables - - yml configuration file structure and value types - - private static class properties (considered to be configuration variables) +If after reading this section you are still unsure what branch your pull request should go to, consider asking either in the GitHub issue that you address with your PR or in one of the various [community channels](https://www.silverstripe.org/community/). -Silverstripe CMS public APIs explicitly exclude: - - private scope (methods and properties with the exception for `private static`) - - entities marked as `@internal` - - yml configuration file default values - - HTML, CSS, JavaScript, TypeScript, SQL and anything else that is not PHP +#### For changes to public API or new/enhanced features -Other entities might be considered to be included or excluded from the public APIs on case-by-case basis. +If you are introducing new APIs, introducing new features, or enhancing an existing feature, you should generally use the default branch of the repository where you want to contribute to. That would usually target the next minor release of the module. -Any updates to third party dependencies in composer.json should aim to target the default branch for a minor release if possible. Targeting a patch release branch is acceptable if updating dependencies is required to fix a bug. +#### For bug fixes that don't introduce new API -API from third party dependencies may indirectly be incorporated into Silverstripe CMS's API if: -- they are defined as a parameter type for a supported method -- they are defined as a return type for a supported method -- they are extended by a Silverstripe CMS class. +If you are fixing a bug that doesn't require API changes, use the highest patch release branch available for the lowest supported major release line the bug applies to. You can see the currently supported release lines for Silverstripe CMS on [the roadmap](https://www.silverstripe.org/software/roadmap/). You can find which major release lines of core and supported modules apply to that version by checking the relevant [/project_governance/supported_modules/](supported modules) page. -When defining a return type or a parameter type, it is preferable to use a more generic interface rather than a specific class. Third party dependencies that are used for internal purposes and are not explicitly exposed via the Silverstripe CMS public API are not covered by SemVer and maybe substituted without notice. +For example, if your bug fix is applicable for Silverstripe CMS 4, and is for the `silverstripe/admin` module, you would target the `1.13` branch. -### The Pull Request Process +#### For API breaking changes -Once your pull request is issued, it's not the end of the road. A [core committer](/project_governance/core_committers/) will most likely have some questions for you and may ask you to make some changes depending on discussions you have. -If you've been naughty and not adhered to the [coding conventions](coding_conventions), -expect a few requests to make changes so your code is in-line. +Do not make a pull request that includes a breaking change, including changing public API (described below), unless there is a major release branch ready to merge into. +e.g. if the latest stable release is `5.2.7`, the major release branch would be `6`. -If your change is particularly significant, it may be referred to the [forum](https://forum.silverstripe.org) for further community discussion. +#### Definition of public API -A core committer will also "label" your PR using the labels defined in GitHub, these are to correctly classify and help find your work at a later date. +Silverstripe CMS public APIs explicitly include (unless excluded below): -#### GitHub Labels {#labels} +- **global** functions, constants, and variables +- namespaces, classes, interfaces, enums, and traits +- public and protected scope (including methods, properties and constants) +- private static class properties (considered to be configuration variables) +- yml configuration file structure and value types +- extension hooks (e.g. `$this->extend('someExtensionHook'));`) -The current GitHub labels are grouped into five sections: +Silverstripe CMS public APIs explicitly exclude: -1. *Impact* - What impact does this bug/issue/fix have, does it break a feature completely, is it just a side effect or is it trivial and not a bit problem (but a bit annoying). Impact is evaluated in the context of the CMS as a whole, rather than against the individual module the issue is raised on. -2. *Complexity* - What level of technical proficiency is required to address this issue? -3. *Type* - The type of solution required to address this issue -4. *Affects* - The release line this issue is relevant to -5. *RFC* - The issue is a request-for-comment +- private scope (with the exception for `private static` properties which aren't annotated with `@internal`) +- all entities marked as `@internal` +- yml configuration file default values +- HTML, CSS, JavaScript (within reason), SQL, and anything else that is not PHP -| Label | Purpose | -| ----- | ------- | -| impact/critical | Website breaking issue with no workarounds. Reserved only for bugs. Bugfix's will target all supported minor release lines. | -| impact/high | Affects a major usage flow. Broken functionality with no obvious workarounds available, or an enhancement that provides a clear benefit to users | -| impact/medium | When affecting a major usage flow, for bugs there is a workaround available and for enhancements there would be a reasonable benefit to users. For a less common usage flow there is broken functionality and for enhancements there is a clear benefit to users. | -| impact/low | A nuisance but doesn't break any functionality (typos, etc). For enhancements there would only be a limited benefit to users. | -| complexity/low | Someone with limited Silverstripe CMS experience could resolve | -| complexity/medium | Someone with a good understanding of Silverstripe CMS could resolve | -| complexity/high | Only an expert with Silverstripe CMS could resolve | -| type/bug | Does not function as intended, or is inadequate for the purpose it was created for | -| type/enhancement | New feature or improvement for either users or developers | -| type/api-break | An API-breaking change requiring a new major release | -| type/ux | Impact on the CMS user interface | -| type/docs | A docs change | -| type/userhelp | A userhelp documentation change | -| affects/* | Issue has been observed on a specific CMS release line | -| rfc/draft | [RFC](/project_governance/request_for_comment) under discussion | -| rfc/accepted | [RFC](/project_governance/request_for_comment) where agreement has been reached | +Other entities might be considered to be included or excluded from the public APIs on case-by-case basis based on how likely it is to cause problems during an upgrade. -### Quickfire Do's and Don't's +Any updates to third party dependencies in composer.json should aim to target the default branch for a minor release if possible. Targeting a patch release branch is acceptable if updating dependencies is required to fix a high impact or critical bug and is unlikely to result in regressions. -If you aren't familiar with git and GitHub, try reading the ["GitHub bootcamp documentation"](https://help.github.com/). -We also found the [free online git book](https://git-scm.com/book/en/v2) and the [git crash course](https://services.github.com/) useful. -If you're familiar with it, here's the short version of what you need to know. Once you fork and download the code: +API from third party dependencies may implicitly be incorporated into our definition of public API if: -* **Don't develop on the master branch.** Always create a development branch specific to "the issue" you're working on (on our [GitHub repository's issues](https://github.com/silverstripe/silverstripe-framework/issues)). Name it by issue number and description. For example, if you're working on Issue #100, a `DataObject::get_one()` bugfix, your development branch should be called 100-dataobject-get-one. If you decide to work on another issue mid-stream, create a new branch for that issue--don't work on both in one branch. +- they are defined as a parameter type for a supported method +- they are defined as a return type for a supported method +- they are extended by a Silverstripe CMS class. -* **Do not merge the upstream master** with your development branch; *rebase* your branch on top of the upstream master. +When defining a return type or a parameter type, it is preferable to use a more generic interface rather than a specific class. Third party dependencies that are used for internal purposes and are not explicitly exposed via the Silverstripe CMS public API are not covered by SemVer and maybe substituted without notice. -* **A single development branch should represent changes related to a single issue.** If you decide to work on another issue, create another branch. +### Step 2: Install the project {#install-the-project} -* **Squash your commits, so that each commit addresses a single issue.** After you rebase your work on top of the upstream master, you can squash multiple commits into one. Say, for instance, you've got three commits in related to Issue #100. Squash all three into one with the message "Description of the issue here (fixes #100)" We won't accept pull requests for multiple commits related to a single issue; it's up to you to squash and clean your commit tree. (Remember, if you squash commits you've already pushed to GitHub, you won't be able to push that same branch again. Create a new local branch, squash, and push the new squashed branch.) +Install the project through composer. The process is described in detail in the [getting started](../getting_started/composer#contributing) docs. -* **Choose the correct branch**: see [Picking the right version](#picking-the-right-version). +```bash +composer create-project --keep-vcs silverstripe/installer ./your-website-folder 4.13.x-dev +``` -### Editing files directly on GitHub.com +Note that if you already have a working project and would like to implement the change in the context of that project, you will need to make sure you have the full source of the module using the [`composer reinstall`](https://getcomposer.org/doc/03-cli.md#reinstall) command: + +```bash +# re-install the module using prefer-source. +# replace / with the module you're making changes to (e.g. silverstripe/framework) +composer reinstall / --prefer-source +``` -If you see a typo or another small fix that needs to be made, and you don't have an installation set up for contributions, you can edit files directly in the github.com web interface. Every file view has an "edit this file" link. +### Step 3: Prepare your working directory {#prepare-your-working-directory} -After you have edited the file, GitHub will offer to create a pull request for you. This pull request will be reviewed along with other pull requests. +- Create a [fork](https://help.github.com/articles/about-forks/) of the module you want to contribute to by going to the repository in your browser, clicking the "fork" button, and following the instructions. -## Check List +- Add your fork as a "remote" to the module you want to contribute to. This is where you will be pushing changes to. -* Adhere to our [coding conventions](/contributing/coding_conventions) -* If your patch is extensive, discuss it first on the [Silverstripe CMS Forums](https://forum.silverstripe.org/c/feature-ideas) (ideally before doing any serious coding) -* When working on existing tickets, provide status updates through ticket comments -* Check your patches against the "master" branch, as well as the latest release branch -* Write [unit tests](../developer_guides/testing/unit_testing) -* Write [Behat integration tests](https://github.com/silverstripe/silverstripe-behat-extension) for any interface changes -* Describe specifics on how to test the effects of the patch -* It's better to submit multiple patches with separate bits of functionality than a big patch containing lots of changes -* Only submit a pull request for work you expect to be ready to merge. Work in progress is best discussed in an issue, or on your own repository fork. -* Document your code inline through [PHPDoc](https://en.wikipedia.org/wiki/PHPDoc) syntax. See our -[API documentation](https://api.silverstripe.org/) for good examples. -* Check and update documentation on [docs.silverstripe.org](https://docs.silverstripe.org). Check for any references to functionality deprecated or extended through your patch. Documentation changes should be included in the patch. -* When introducing something "noteworthy" (new feature, API change), [update the release changelog](/changelogs) for the next release this commit will be included in. -* If you get stuck, please post to the [forum](https://www.silverstripe.org/community/forums) -* When working with the CMS, please read the ["CMS Architecture Guide"](/developer_guides/customising_the_admin_interface/cms_architecture) first -* Try to respond to feedback in a timely manner. PRs that go more than a month without a response from the author are considered stale, and will be politely chased up. If a response still isn't received, the PR will be closed two weeks after that. + ```bash + cd vendor// + git remote add pr git@github.com:/.git + ``` -## Commit Messages +- Create a working branch. -We try to maintain a consistent record of descriptive commit messages. + ```bash + # make sure you're starting from the correct branch first + cd vendor// + git checkout --track origin/4.13 + # then create your working branch + git checkout -b + ``` + +[hint] +Use a descriptive name for your branch. For example if you are fixing a bug related to swapping preview modes targetting the `4.13` branch: `pulls/4.13/fix-preview-modes` +[/hint] + +### Step 4: Work on your pull request {#work-on-your-pr} + +Work on the code as much as you want and commit as often as you need to, but keep the following in mind: + +- Adhere to our [coding conventions](/contributing/coding_conventions) +- Most pull requests only need a single commit, but complex changes might be better served with multiple commits. In either case, each commit must have a clear and distinct purpose +- Commit messages should conform to our [commit message guidelines](#commit-messages) +- Document new API through [PHPDoc](https://en.wikipedia.org/wiki/PHPDoc) comments. These are used in IDEs and in our [API documentation](https://api.silverstripe.org/). +- Add [unit tests](../developer_guides/testing/unit_testing) which prove your change works and which will prevent future regressions +- Add [behat tests](https://github.com/silverstripe/silverstripe-behat-extension) for any complex behaviour and for any JavaScript functionality changes +- Avoid making unrelated changes (such as fixing coding standards) which are not the focus of your pull request. Those sorts of changes increase the work required to review your pull request. +- It's better to submit multiple pull requests with separate bits of functionality than a big pull request containing lots of changes. If your pull request contains lots of unrelated changes you will be asked to submit them as separate pull requests. +- If you are adding a new feature or changing the way an existing feature behaves, you will need to also create a pull request against [silverstripe/developer-docs](https://github.com/silverstripe/developer-docs) to update the documentation and add information about the change to the changelog. +- If you have updated any source files for CSS or JavaScript, you'll need to build and commit the dist files. See [client-side build tooling](build_tooling) for details. + +#### Commit messages + +We try to maintain a consistent record of descriptive commit messages. Most importantly: Keep the first line short, and add more detail below. -This ensures commits are easy to browse, and look nice on github.com -(more info about [proper git commit messages](http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html)). +This ensures commits are easy to browse and quickly see what the purpose of the commit is. -Our [changelog](https://docs.silverstripe.org/en/changelogs/) generation tool relies upon commit prefixes (tags) -to categorize the patches accordingly and produce more readable output. Prefixes are usually a single case-insensitive word, -at the beginning of the commit message. Although prefixing is optional, **noteworthy** patches should have them to fall into -correct categories. +Our [changelog](https://docs.silverstripe.org/en/changelogs/) generation tool relies upon commit prefixes +to categorize commits and produce more readable output. The prefixes are a single case-insensitive term +at the beginning of the commit message. | Prefix | Description | | --- | --- | -| API | Addition of a new API, or modification/removal/deprecation of an existing API. Includes any change developers should be aware of when upgrading. | -| FIX | Bugfix on something developers or users are likely to encounter. | -| DOC | Any documentation changes. | +| API | Addition of a new public/protected API, or modification/removal/deprecation of an existing API. | | NEW | New feature or major enhancement (both for users and developers) | | ENH | Improvements of existing functionality (with no API changes), UI/UX enhancements, refactoring and configuration updates. | -| MNT | Maintenance commits that have no impact on users and applications (e.g. CI configs) | +| FIX | Bugfix on something developers or users are likely to encounter. | +| DOC | Any documentation changes. | | DEP | Dependency version updates (updates for composer.json, package.json etc) | -| Merge | PR merges and merge-ups | - -If you can't find the correct prefix for your commit, it is alright to leave it untagged, then it will fall into "Other" category. +| MNT | Maintenance commits that have no impact on users and applications (e.g. CI configs) - ommitted from the changelog | +| Merge | PR merges and merge-ups - ommitted from the changelog | -Further guidelines: +If you can't find the correct prefix for your commit, it is alright to leave it untagged. The commit will then fall into "Other" category. -* Each commit should form a logical unit - if you fix two unrelated bugs, commit each one separately -* If you are fixing a issue from our bugtracker (see [Reporting Bugs](issues_and_bugs)), please append `(fixes #)` -* When fixing issues across repos (e.g. a commit to `framework` fixes an issue raised in the `cms` bugtracker), - use `(fixes silverstripe/silverstripe-cms#)` ([details](https://github.com/blog/1439-closing-issues-across-repositories)) -* If your change is related to another commit, reference it with its abbreviated commit hash. -* Mention important changed classes and methods in the commit summary. +Example: Good commit message -Example: Bad commit message +```text +FIX Allow multiple iterations of eager-loaded DataLists -``` -finally fixed this dumb rendering bug that Joe talked about ... LOL -also added another form field for password validation +Previously, a second iteration would add every relation item to the +relation list a second time - so with each iteration your relation list +count doubled (though it was the same records time and again). ``` -Example: Good commit message +### Step 5: Create the pull request {#create-the-pr} +When you are ready, push your branch to your GitHub fork. It's also a good idea to do this during development if the process is taking more than one day since this effectively backs up your work for you. + +Only submit a pull request for work you think is ready to merge. Work in progress is best discussed in an issue (you can link to the code in your fork if you need to refer to it). + +```bash +cd vendor// +git push pr ``` -FIX Formatting through prepValueForDB() -Added prepValueForDB() which is called on DBField->writeToManipulation() -to ensure formatting of value before insertion to DB on a per-DBField type basis (fixes #1234). -Added documentation for DBField->writeToManipulation() (related to a4bd42fd). +Then [create a pull request](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request-from-a-fork) on GitHub. If you are raising multiple pull requests that work together to solve a problem, make sure you link to them and indicate what the dependencies are (e.g. one PR might require another in order to work and for the tests to pass - in that case we need to know which to merge first). + +If there is a template for the pull request description, follow it as closely as you can. If there isn't, then provide a quick summary of what your changes are, why you're making them, and a link to the relevant GitHub issue. Make sure to include steps to manually test the effects of the change. + +It's also a good idea to add a link to your PR in the relevant GitHub issue. Add the link in the issue description if you have edit rights, and in a new comment otherwise. Doing this improves the chances of your PR being noticed. + +Following these additional guidelines for your pull request will improve the chances that your change will be merged: + +- Link to [the relevant GitHub issue](#make-or-find-a-github-issue) from your pull request description +- Link to your pull request from that issue (in the description if you can edit it, or in a comment otherwise) +- Explain your implementation. If there's anything which you needed to do a deep dive to find the best way to do it, or anything potentially controversial, etc, you can add a comment to your own pull request explaining what you did and why you did it that way. + +### Step 6: Recieve and respond to feedback {#recieve-feedback} + +Once your pull request is created, it's not the end of the road. + +#### Automated feedback + +Most of the core and supported repositories have an automated GitHub Actions workflow which will run on your pull request. When it finishes running, check for any failed builds. + +If you think a build has failed for reasons unrelated to the changes you've made, point that out in a comment. If the failure _is_ related to your changes, then make any adjustments necessary to resolve the problems. + +#### Peer review feedback + +The core team will review the pull request as time permits. They will most likely have some questions for you and may ask you to make some changes, so make sure you have [configured your GitHub notifications](https://docs.github.com/en/account-and-profile/managing-subscriptions-and-notifications-on-github/setting-up-notifications/configuring-notifications) appropriately. + +- Try to respond to feedback in a timely manner. PRs that go for a while without a response from the author are considered stale, and will be politely chased up. If a response still isn't received, the PR will eventually be closed. +- If you don't agree with a requested change, provide a clear reason why. Bonus points for showing precedent in the existing codebase. But be open to accepting alternative view points - if a member of the core team insists that you make the change after responding to your reasoning, it's often best to defer to their judgment. + +#### Resolving merge conflicts + +If other changes are merged in before yours, your pull request may end up with merge conflicts. You'll need to resolve those by rebasing your branch on top of the target branch, and then manually resolving the merge conflicts. + +[warning] +Using `--force-with-lease` is necessary after a rebase, because otherwise GitHub will reject your push. This is because your commit hashes will have changed. But beware that you are explicitly telling GitHub that you are intentionally overriding data. Make sure you have the correct branch name when doing that step to avoid accidentally overriding other branches in your forked repository. +[/warning] + +```bash +cd vendor// +git checkout 4.13 +git pull +git checkout +git rebase 4.13 +# if there are merge conflicts, resolve them at this stage then run git rebase --continue +git push pr --force-with-lease ``` + +#### GitHub Labels {#labels} + +The current GitHub labels are grouped into five sections: + +1. *Impact* - What impact does this bug/issue/fix have, does it break a feature completely, is it just a side effect or is it trivial and not a bit problem (but a bit annoying). Impact is evaluated in the context of the CMS as a whole, rather than against the individual module the issue is raised on. +2. *Complexity* - What level of technical proficiency is required to address this issue? +3. *Type* - The type of solution required to address this issue +4. *Affects* - The release line this issue is relevant to +5. *RFC* - The issue is a request-for-comment + +| Label | Purpose | +| ----- | ------- | +| impact/critical | Website breaking issue with no workarounds. Reserved only for bugs. Bugfix's will target all supported minor release lines. | +| impact/high | Affects a major usage flow. Broken functionality with no obvious workarounds available, or an enhancement that provides a clear benefit to users | +| impact/medium | When affecting a major usage flow, for bugs there is a workaround available and for enhancements there would be a reasonable benefit to users. For a less common usage flow there is broken functionality and for enhancements there is a clear benefit to users. | +| impact/low | A nuisance but doesn't break any functionality (typos, etc). For enhancements there would only be a limited benefit to users. | +| complexity/low | Someone with limited Silverstripe CMS experience could resolve | +| complexity/medium | Someone with a good understanding of Silverstripe CMS could resolve | +| complexity/high | Only an expert with Silverstripe CMS could resolve | +| type/bug | Does not function as intended, or is inadequate for the purpose it was created for | +| type/enhancement | New feature or improvement for either users or developers | +| type/api-break | An API-breaking change requiring a new major release | +| type/ux | Impact on the CMS user interface | +| type/docs | A docs change | +| type/userhelp | A userhelp documentation change | +| affects/* | Issue has been observed on a specific CMS release line | +| rfc/draft | [RFC](/project_governance/request_for_comment) under discussion | +| rfc/accepted | [RFC](/project_governance/request_for_comment) where agreement has been reached |