Skip to content
This repository has been archived by the owner on Jul 28, 2022. It is now read-only.

Latest commit

 

History

History
298 lines (221 loc) · 12.1 KB

CONTRIBUTING.md

File metadata and controls

298 lines (221 loc) · 12.1 KB
Raw

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.

Summary

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, 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.

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. 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! 😄

But please, read the following before.

Content

Coding style

Each project follows PSR-1, PSR-2 and Symfony Coding Standards for coding style, PSR-4 for autoloading.

Please install PHP Coding Standard Fixer and run this command before committing your modifications:

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 pseudo-types, generics, array shapes or 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.

/**
 * @param Bar|Baz $foo
 * @param int     $limit a crucial, highly interesting comment
 *
 * @phpstan-param class-string $class
 */
protected function bar($foo, string $class, int $limit)
{
    // ...
}

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.

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 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:

$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.

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 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().

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

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:

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.

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. 😉)