<NME> CONTRIBUTING.md
<BEF> <!--
DO NOT EDIT THIS FILE!

It's auto-generated by sonata-project/dev-kit package.
-->

# Sonata project contribution

Thanks for your interest in Sonata projects!

This document is about issues and pull requests.

The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD",
"SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this document are to be
interpreted as described in [RFC 2119](https://www.ietf.org/rfc/rfc2119.txt).

## Summary

* [Issues](#issues)
* [Pull Requests](#pull-requests)
    * [Content](#content)
        * [Coding style](#coding-style)
        * [Documentation](#documentation)
        * [Tests](#tests)
        * [Frontend](#frontend)
    * [Writing a pull request](#writing-a-pull-request)
        * [Subject](#subject)
        * [Changelog](#changelog)
        * [Base branch](#base-branch)
        * [Commit messages](#commit-messages)

## Issues

First, check if you are up to date: is your version still supported, and are
you using the latest patch version?

GitHub Issues is for **issues**, as opposed to question on how to use Sonata.
If you are not sure this is a bug, or simply want to ask such a question,
please post your question on [Stack Overflow](http://stackoverflow.com/questions/tagged/sonata),
using the `sonata` tags.

If you happen to find a bug, we kindly request you report it. However,
before submitting it, please check the [project documentation available
online](https://sonata-project.org/bundles/).

Then, if it appears that it is indeed a real bug, you MAY report it using
Github by following these points are taken care of:

* Check if the bug is not already reported!
* The title sums up the issue with clarity.
* A description of the workflow needed to reproduce the bug. Please try to make
  sentences, dumping an error message by itself is frowned upon.
* If your issue is an error page, you MUST provide us with a stack trace. With
  recent versions of Symfony, you can even get stack traces as plain text at the
end of the page. Just look for "Stack Trace (Plain Text)", and copy/paste what
you see. **Do not** make a screenshot of the stack trace, as screenshots are
not indexed by search engines and will make it difficult for other people to
find your bug report. If you have an issue when using the Symfony CLI,
use the `-vvv` option to get a stack trace.
* Screenshots SHOULD be considered additional data, and therefore, you SHOULD
  always provide a textual description of the bug. It is strongly RECOMMENDED
to provide them when reporting UI-related bugs.
* If you need to provide code, make sure you know how to get syntactic
  coloration, in particular with [fenced code
blocks](https://help.github.com/articles/creating-and-highlighting-code-blocks/).
When you feel the code is to long, use external code pastebin like
https://gist.github.com/ or http://hastebin.com/ . If this is not sufficient,
just create a repository to show the issue.

> _NOTE:_ Don't hesitate to give as much information as you can (OS, PHP
> version, extensions...)

## Pull Requests

All the sonata team will be glad to review your code changes propositions! :smile:

But please, read the following before.

### Content

#### Coding style

Each project follows [PSR-1](http://www.php-fig.org/psr/psr-1/), [PSR-2](http://www.php-fig.org/psr/psr-2/)
and [Symfony Coding Standards](http://symfony.com/doc/current/contributing/code/standards.html) for coding style,
[PSR-4](http://www.php-fig.org/psr/psr-4/) for autoloading.

Please [install PHP Coding Standard Fixer](http://cs.sensiolabs.org/#installation)
and run this command before committing your modifications:

```bash
php-cs-fixer fix --verbose
```

Please note that we try to keep phpdoc to a minimum, so if an `@param` phpdoc
comment brings nothing more than the type hint and variable name already do,
it SHOULD be removed. Descriptions are OPTIONAL if you want to document a type.

If you want to use templates for static-analysis, prefer the usage of
`@phpstan-` annotations over classic annotations to keep IDE support, and
over `@psalm-` annotations for consistency. Keep standard annotations if necessary.

```php
/**
 * @param Bar|Baz      $foo
 * @param class-string $class
 * @param int          $limit a crucial, fascinating comment
 *
 * @return object
 *
 * @phpstan-template T of object
 * @phpstan-param class-string<T> $class
 * @phpstan-return T
 */
protected function bar($foo, string $class, int $limit): object
{
    // ...
}
```

Please also note that multiline signatures are allowed when the line is longer than 120 characters.

#### Documentation

The documentation is mostly written with the `rst` format, and can be found in the `docs` directory.
You can test the doc rendering with the `make docs` command, but to do this, you will need [Sphinx][sphinx_install].
Just like php dependencies can be managed with Composer, python dependencies can be managed with [pip][pip_install].
To get sphinx, simply run the following command.

```bash
pip install --requirement docs/requirements.txt --user
```

Some python binaries SHOULD be downloaded to `~/.local/bin` for Linux or `~/Library/Python/2.7/bin` for Mac OS,
[modify your `$PATH` environment variable](http://www.linfo.org/path_env_var.html)
so that it contains this path and then, from the root of the project, run `make docs`

If `make docs` is successful, you SHOULD be able to see your modifications:

```bash
$YOUR_FAVORITE_BROWSER docs/_build/html/index.html
```

If your PR contains a new feature, you MUST add documentation for it.
Of course, you can also create PRs consisting only in documentation changes.

Documentation contributions SHOULD comply with [the Symfony documentation standards](https://symfony.com/doc/current/contributing/documentation/standards.html).

#### Tests

If your PR contains a fix, tests SHOULD be added to prove the bug.

If your PR contains an addition, a new feature, this one has to be fully covered by tests.

Some rules have to be respected about the test:

* Prefer [the built-in test doubles implementation](https://phpunit.de/manual/current/en/test-doubles.html) over prophecy.
* Annotations about coverage are prohibited. This concerns:
  * `@covers`
  * `@coversDefaultClass`
  * `@coversNothing`
  * `@codeCoverageIgnore`
  * `@codeCoverageIgnoreStart`
  * `@codeCoverageIgnoreEnd`
* All test methods MUST be prefixed by `test`. Example: `public function testItReturnsNull()`.
* As opposed, the `@test` annotation is prohibited.
* All test method names MUST be in camel case format.
* Most of the time, the test class SHOULD have the same name as the targeted class, suffixed by `Test`.
* The `@expectedException*` annotations are prohibited. Use `PHPUnit_Framework_TestCase::setExpectedException()`.

#### Frontend

Frontend assets are all located under `assets` directory, with the following structure:

* `assets/js` for the JavaScript files
* `assets/scss` for the [SCSS](https://sass-lang.com/) files
* `assets/images` for the images
* `assets/vendor` for the third party assets that can't be loaded via [NPM](https://www.npmjs.com/)

This `assets` directory contains only the source code, and it is not directly accesible for the
website, because is not placed under the `src/Resources/public`.

If you take a look at the `src/Resources/public` you will see a lot of minified assets, this is done with
[Webpack](https://webpack.js.org/), we are using [Webpack Encore](https://github.com/symfony/webpack-encore) to configure and run it.

Similar to what Composer is for PHP there is a package manager for the frontend stack, called NPM and we declare our
dependencies on `package.json` and the lockfile is `package-lock.json`, because we are using [NPM](https://www.npmjs.com/).
It is common to commit the lockfile in the case of frontend, because dependencies tend to upgrade really often and it
affects directly the compiled files.

If you PR is focused on fixing or improving the frontend you might need to modify
JavaScript or CSS code. To do so, you will first need to install the dependencies:

```bash
npm clean-install

# if you want to upgrade some package you SHOULD run instead
npm install
```

To compile assets before the pull request you should run:

```bash
npx encore production
```

This command will make the lint checks and compile for production usage the assets. You can also run
lint checks ([ESLint](https://eslint.org/) and [Stylelint](https://stylelint.io/)) with:

```bash
npx eslint assets/js
npx stylelint assets/scss
```

We have some strict rules following the most popular coding standards for JavaScript and SCSS.

* [Airbnb JavaScript Style Guide](https://github.com/airbnb/javascript)
* [Stylelint Recommended Configuration](https://github.com/stylelint/stylelint-config-recommended)

We also use [Prettier](https://prettier.io/) to do the code formatting for all the `assets` folder, to run it manually:

```bash
npx prettier --write assets
```

You can also configure your IDE to do it automatically on save:

* [PHPStorm](https://www.jetbrains.com/help/phpstorm/prettier.html)
* [VSCode](https://marketplace.visualstudio.com/items?itemName=esbenp.prettier-vscode)

In case you need more documentation before making your Pull Request, please consider reading
the documentation of all mentioned tools.

### Writing a Pull Request

#### Subject

Ideally, a Pull Request SHOULD concern one and **only one** subject, so that it
remains clear, and independent changes can be merged quickly.

If you want to fix a typo and improve the performance of a process, you SHOULD
try as much as possible to do it in a **separate** PR, so that we can quickly
merge one while discussing the other.

The goal is to have a clear commit history and make a possible revert easier.

If you found an issue/typo while writing your change that is not related to
your work, please do another PR for that. In some rare cases, you might be
forced to do it on the same PR. In this kind of situation, please add a comment
on your PR explaining why you feel it is the case.

#### Changelog

For each PR, a change log MUST be provided.

There are few cases where no change log is necessary:

* When you fix a bug on an unreleased feature.
* When your PR concerns only the documentation (fix or improvement).

**Do not** edit the `CHANGELOG.md` directly though, because having every
contributor write PR with changes in the same file, at roughly the same line is
a recipe for conflicts. Instead, fill in the dedicated section that SHOULD
appear in a textaread when submitting your PR.

Your note can be put on one of these sections:

* `Added` for new features.
* `Changed` for changes in existing functionality.
* `Deprecated` for deprecation of features that will be removed in next major release.
* `Removed` for deprecated features removed in this release.
* `Fixed` for any bug fixes.
* `Security` to invite users to upgrade in case of vulnerabilities.

More information about the followed changelog format: [keepachangelog.com](http://keepachangelog.com/)

#### Base branch

Before writing a PR, you have to check on which branch your changes SHOULD be based.

Each project follows [semver](http://semver.org/) convention for release management.

Here is a short table resuming on which you have to start:

Kind of modification | Backward Compatible (BC) | Type of release | Branch to target        | Label |
-------------------- | ------------------------ | --------------- | ----------------------- | ----- |
Bug fixes            | Yes                      | Patch           | `4.x`   |       |
Bug fixes            | No (Only if no choice)   | Major           | `5.x` |       |
Feature              | Yes                      | Minor           | `4.x`   |       |
Feature              | No (Only if no choice)   | Major           | `5.x` |       |
Deprecation          | Yes (Have to)            | Minor           | `4.x`   |       |
Deprecation removal  | No (Can't be)            | Major           | `5.x` |       |

Notes:
  * Branch `4.x` is the branch of the **latest stable** minor release and
  has to be used for Backward compatible PRs.
  * If you PR is not **Backward Compatible** but can be, it **MUST** be:
    * Changing a function/method signature? Prefer create a new one and deprecate the old one.
    * Code deletion? Don't. Please deprecate it instead.
    * If your BC PR is accepted, you can do a new one on the `5.x` branch which removes the deprecated code.
    * SYMFONY DOC REF (same logic)?

If you have a non-BC PR to propose, please try to create a related BC PR first.
This BC PR SHOULD mark every piece of code that needs to be removed / uncommented / reworked
in the corresponding non-BC PR with the following marker comment : `NEXT_MAJOR`.
When the BC PR is merged in the stable branch, wait for the stable branch to be
merged in the unstable branch, and then work on your non-BC PR.

For instance, assuming you want to introduce a new method to an existing interface, you SHOULD do something like this:

```php
<?php

namespace Foo;

/**
 * @method void usefulMethod()
 */
interface BarInterface
{
  /**
   * NEXT_MAJOR: Uncomment this method
   *
   * This method does useful stuff.
   */
  // public function usefulMethod(): void;

  // …
}
```

##### Deprecation

In some cases, you might have some code parts that might become superseded by others, but could still be used by the end user.
If the concerned code is not tagged as `internal`, it MUST be deprecated on the stable branch, then removed.

If an alternate usage solution is possible, you **MUST** provide it in the deprecation message.

The deprecated minor version **MUST NOT** be provided. Use `x` instead. It will be updated when releasing.

Any deprecation **MUST** be documented in the corresponding `UPGRADE-[0-9].x.md`.
The documentation **MUST** be filled inside the top **unreleased** section with a sub title.

The `NEXT_MAJOR` tag SHOULD also be used for deprecations, it will be searched for before releasing the next major version.

You have three ways to deprecate things.

For class definitions and properties, use the `@deprecated` tag.
For methods, use the `@deprecated` tag and trigger a deprecation with `@trigger_error('...', \E_USER_DEPRECATED)`:

```php
/**
 * NEXT_MAJOR: Remove this class.
 *
 * @deprecated since sonata-project/foo-lib 42.x, to be removed in 43.0. Use Shiny\New\ClassOfTheMonth instead.
 */
final class IAmOldAndUseless
{
}

final class StillUsedClass
{
    /**
     * NEXT_MAJOR: Remove this property.
     *
     * @deprecated since sonata-project/foo-lib 42.x, to be removed in 43.0.
     */
    public $butNotThisProperty;

    /**
     * NEXT_MAJOR: Remove this method.
     *
     * @deprecated since sonata-project/foo-lib 42.x, to be removed in 43.0.
     */
    public function iAmBatman()
    {
        @trigger_error(sprintf(
            'Method %s() is deprecated since sonata-project/foo-lib 42.x and will be removed in version 43.0.',
            __METHOD__
        ), \E_USER_DEPRECATED);

        echo "But this is not Gotham here.";
    }
}
```

If the deprecated thing is a service, you **MUST** specify it on the service definition:

```xml
<!-- NEXT_MAJOR: Remove this service -->
<service id="sonata.block.old" class="Sonata\Block\Old">
    <argument type="service" id="security.token_storage" />
    <deprecated>The "%service_id%" service is deprecated since sonata-project/bar-bundle 42.x and will be removed in 43.0.</deprecated>
 </service>
 ```

More info: http://symfony.com/blog/new-in-symfony-2-8-deprecated-service-definitions

For everything else, not managed by the `@deprecated` tag,
you **MUST** still trigger a deprecation message (and add a `NEXT_MAJOR` comment).

```php
<?php
// NEXT_MAJOR: Remove this condition.
if (/* some condition showing the user is using the legacy way */) {
    @trigger_error(
        'This is deprecated since sonata-project/bar-bundle 42.x and will not be supported in version 43.0.',
        \E_USER_DEPRECATED
    );
} else {
    // new way of doing things
}
```

In the case of a deprecation, unit tests might show your deprecation notice.
You **MUST** mark such tests with the `@group legacy` annotation and add a `NEXT_MAJOR` comment to explain
how to deal with them in the next major (with a removal or some changes).

Be aware that pull requests with BC breaks could be rejected
or postponed to next major release **only** if BC is not possible.

If you are not sure what should be done, don't hesitate to open an issue about your PR project.

##### Dependency changes

If you want to change some dependencies, here are the rules:

- Don't add support for a version lower than the current one.
- Don't change the highest supported version to a lower one.
- Lower version dropping is accepted as a Backward Compatible change according to [semver][semver_dependencies_update],
but some extra rules MUST be respected here:
  - PHP versions that are under the [orange zone][php_supported_versions] (Security Support) **MUST NOT** be dropped on the stable branch.
  - PHP versions that are under the [green zone][php_supported_versions] (Active Support) **MUST NOT** be dropped on the unstable branch.
  - If it's a Symfony package, at least the last LTS version **MUST** be supported, even on the unstable branch.
  - Generally, don't drop dependency version if it doesn't have a big impact on the code.
  - Backward Compatible code related to the dropped version **MUST** be dropped on the same PR.
    This will allow seeing if this version drop **is really worth it** or not.
    Please note that we can refuse a version drop at any moment if the gain does not seem sufficient.

##### Legacy branches

Legacy branches are **NOT** supported at all. Any submitted Pull Request will be immediately closed.

Core team members *MAY* cherry-pick some fixes from the stable branch to the legacy one if it's really needed
and if the legacy one is not too old (~less than one month).

#### Commit messages

Sonata is a big project with many contributors, and a big part of the job is
being able to understand the code at all times, be it when submitting a PR or
looking at the history. Good commit messages are crucial to achieve this goal.

There are already a few articles (or even single purpose websites) about this,
we cannot recommend enough the following:

* http://rakeroutes.com/blog/deliberate-git
* http://stopwritingramblingcommitmessages.com
* http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html

To sum them up, the commit message has to be crystal clear and of course,
related to the PR content.

The first line of the commit message MUST be short, keep it under 50
characters. It MUST say concisely but *precisely* what you did. The other
lines, if needed, SHOULD contain a complete description of *why* you did this.

Bad commit message subject:

```
Update README.md
```

Good commit message subject :

```
Document how to install the project
```

Also, when you specify what you did avoid commit message subjects with "Fix bug
in such and such feature". Saying you are fixing something implies the previous
implementation was wrong and yours is right, which might not even be true.
Instead, state unquestionable technical facts about your changes, not opinions.
Then, in the commit description, explain why you did that and how it fixes
something.
```
call foo::bar() instead of bar::baz()

This fixes a bug that arises when doing this or that, because baz() needs a
flux capacitor object that might not be defined.
Fixes #42
```

The description is OPTIONAL but strongly RECOMMENDED. It could be asked by the
team if needed. PR will often lead to complicated, hard-to-read conversations
with many links to other web pages.

The commit description SHOULD be able to live without what is said in the PR,
and SHOULD ideally sum it up in a crystal clear way, so that people do not have
to open a web browser to understand what you did.
Links to PRs/Issues and external references are of course welcome, but SHOULD
not be considered enough. When you reference an issue, make sure to use one of
the keywords described in [the dedicated github
article](https://help.github.com/articles/closing-issues-via-commit-messages/).

Good commit message with description :

```
Change web UI background color to pink

This is a consensus made on #4242 in addition to #1337.

We agreed that blank color is boring and so deja vu. Pink is the new way to do.
```
(Obviously, this commit is fake. :wink:)

#### Upgrading code comments and deprecation messages

All occurences of `42.x` in comments or deprecation messages should be updating
by resolving `x` to its value at the time of the release.

:warning: You can do it quickly with a "Search and Replace" feature, but be careful not to replace unwanted matches.

<MSG> DevKit updates (#6051)


<DFF> @@ -615,7 +615,7 @@ that should hopefully be there, to a new section named `UPGRADE FROM 42.3.1 to 4
 
 #### Upgrading code comments and deprecation messages
 
-All occurences of `42.x` in comments or deprecation messages should be updating
+All occurrences of `42.x` in comments or deprecation messages should be updating
 by resolving `x` to its value at the time of the release.
 
 :warning: You can do it quickly with a "Search and Replace" feature, but be careful not to replace unwanted matches.

{'addition_count': 1, 'commit_subject': 'DevKit updates (#6051)', 'deletion_count': 1, 'file_extension': '.md', 'lang': 'md', 'license': 'mit', 'repo_name': 'sonata-project/SonataAdminBundle'}