|
@@ -1,6 +1,6 @@
|
|
# Sonata project contribution
|
|
# Sonata project contribution
|
|
|
|
|
|
-Thanks for you interest onto Sonata projects!
|
|
|
|
|
|
+Thanks for your interest in Sonata projects!
|
|
|
|
|
|
## Summary
|
|
## Summary
|
|
|
|
|
|
@@ -15,7 +15,7 @@ you using the latest patch version?
|
|
|
|
|
|
If you are not sure this is a bug, consider posting your question on [Stack
|
|
If you are not sure this is a bug, consider posting your question on [Stack
|
|
Overflow](http://stackoverflow.com), using one of the sonata tags.
|
|
Overflow](http://stackoverflow.com), using one of the sonata tags.
|
|
-If you happen to find a bug, we kindly request you to report it. However,
|
|
|
|
|
|
+If you happen to find a bug, we kindly request you report it. However,
|
|
before submitting it, please check the [project documentation available
|
|
before submitting it, please check the [project documentation available
|
|
online](https://sonata-project.org/bundles/).
|
|
online](https://sonata-project.org/bundles/).
|
|
|
|
|
|
@@ -23,10 +23,10 @@ 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:
|
|
Github by following these points are taken care of:
|
|
|
|
|
|
* Check if the bug is not already reported!
|
|
* Check if the bug is not already reported!
|
|
-* A clear title to sum up the issue
|
|
|
|
|
|
+* The title sums up the issue with clarity.
|
|
* A description of the workflow needed to reproduce the bug. Please try to make
|
|
* A description of the workflow needed to reproduce the bug. Please try to make
|
|
- sentence, dumping an error message by itself is not great.
|
|
|
|
-* If your issue is an error page, you must provide us with a stack trace. With
|
|
|
|
|
|
+ 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
|
|
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
|
|
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
|
|
you see. **Do not** make a screenshot of the stack trace, as screenshots are
|
|
@@ -42,7 +42,7 @@ 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,
|
|
https://gist.github.com/ or http://hastebin.com/ . If this is not sufficient,
|
|
just create a repository to show the issue.
|
|
just create a repository to show the issue.
|
|
|
|
|
|
-> _NOTE:_ Don't hesitate giving as much information as you can (OS, PHP
|
|
|
|
|
|
+> _NOTE:_ Don't hesitate to give as much information as you can (OS, PHP
|
|
> version, extensions...)
|
|
> version, extensions...)
|
|
|
|
|
|
## Pull Requests
|
|
## Pull Requests
|
|
@@ -86,7 +86,7 @@ Some rules have to be respected about the test:
|
|
* `@coversNothing`
|
|
* `@coversNothing`
|
|
* `@codeCoverageIgnore`
|
|
* `@codeCoverageIgnore`
|
|
* `@codeCoverageIgnoreStart`
|
|
* `@codeCoverageIgnoreStart`
|
|
- * `codeCoverageIgnoreEnd`
|
|
|
|
|
|
+ * `@codeCoverageIgnoreEnd`
|
|
* All test methods should be prefixed by `test`. Example: `public function testItReturnsNull()`.
|
|
* All test methods should be prefixed by `test`. Example: `public function testItReturnsNull()`.
|
|
* As opposed, the `@test` annotation is prohibited.
|
|
* As opposed, the `@test` annotation is prohibited.
|
|
* Most of the time, the test class should have the same name as the targeted class, suffixed by `Test`.
|
|
* Most of the time, the test class should have the same name as the targeted class, suffixed by `Test`.
|
|
@@ -99,10 +99,10 @@ Ideally, a Pull Request should concern one and **only one** subject, so that it
|
|
remains clear, and independent changes can be merged quickly.
|
|
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
|
|
If you want to fix a typo and improve the performance of a process, you should
|
|
-try as much as possible to it in a **separate** PR, so that we can quickly
|
|
|
|
|
|
+try as much as possible to do it in a **separate** PR, so that we can quickly
|
|
merge one while discussing the other.
|
|
merge one while discussing the other.
|
|
|
|
|
|
-The goal is to have a clear commit history and make possible revert easier.
|
|
|
|
|
|
+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
|
|
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
|
|
your work, please do another PR for that. In some rare cases, you might be
|
|
@@ -155,9 +155,9 @@ Notes:
|
|
* Branch `3.x` is the branch of the **latest stable** minor release and
|
|
* Branch `3.x` is the branch of the **latest stable** minor release and
|
|
has to be used for Backward compatible PRs.
|
|
has to be used for Backward compatible PRs.
|
|
* If you PR is not **Backward Compatible** but can be, it **must** be:
|
|
* If you PR is not **Backward Compatible** but can be, it **must** be:
|
|
- * Changing a function/method signature? Prefer create a new one and deprecated the old one.
|
|
|
|
|
|
+ * Changing a function/method signature? Prefer create a new one and deprecate the old one.
|
|
* Code deletion? Don't. Please deprecate it instead.
|
|
* Code deletion? Don't. Please deprecate it instead.
|
|
- * If your BC PR is accepted, you can do a new one on the `master` branch which remove the deprecated code.
|
|
|
|
|
|
+ * If your BC PR is accepted, you can do a new one on the `master` branch which removes the deprecated code.
|
|
* SYMFONY DOC REF (same logic)?
|
|
* SYMFONY DOC REF (same logic)?
|
|
|
|
|
|
If you have a non-BC PR to propose, please try to create a related BC PR first.
|
|
If you have a non-BC PR to propose, please try to create a related BC PR first.
|
|
@@ -186,10 +186,29 @@ interface BarInterface
|
|
}
|
|
}
|
|
```
|
|
```
|
|
|
|
|
|
-Be aware that pull requests with BC breaks could be not accepted
|
|
|
|
-or reported for next major release if BC is not possible.
|
|
|
|
|
|
+In some cases, you will have the possibility to warn the user that things will change,
|
|
|
|
+and recommend a new way of doing things. You can do so by triggering the dedicated kind of error, like this:
|
|
|
|
|
|
-If you are not sure of what to do, don't hesitate to open an issue about your PR project.
|
|
|
|
|
|
+```php
|
|
|
|
+<?php
|
|
|
|
+if (/* some condition showing the user is using the legacy way */) {
|
|
|
|
+ @trigger_error(
|
|
|
|
+ 'The '.__METHOD__.' method is deprecated since 42.x, to be removed in 43.0. '.
|
|
|
|
+ 'Use FooClass::barMethod() instead.',
|
|
|
|
+ E_USER_DEPRECATED
|
|
|
|
+ );
|
|
|
|
+} else {
|
|
|
|
+ // new way of doing things
|
|
|
|
+}
|
|
|
|
+```
|
|
|
|
+
|
|
|
|
+In that case, unit tests might show your deprecation notice. You must mark such tests with the `@group legacy` annotation,
|
|
|
|
+and if need be, isolate them in a new test method that can simply be removed in the non-BC PR.
|
|
|
|
+
|
|
|
|
+Be aware that pull requests with BC breaks could be rejected
|
|
|
|
+or postponed to next major release 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.
|
|
|
|
|
|
#### The commit message
|
|
#### The commit message
|
|
|
|
|
|
@@ -225,7 +244,7 @@ Document how to install the project
|
|
|
|
|
|
Also, when you specify what you did avoid commit message subjects with "Fix bug
|
|
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
|
|
in such and such feature". Saying you are fixing something implies the previous
|
|
-implementation was wrong and yours is right, which might not be even true.
|
|
|
|
|
|
+implementation was wrong and yours is right, which might not even be true.
|
|
Instead, state unquestionable technical facts about your changes, not opinions.
|
|
Instead, state unquestionable technical facts about your changes, not opinions.
|
|
Then, in the commit description, explain why you did that and how it fixes
|
|
Then, in the commit description, explain why you did that and how it fixes
|
|
something.
|
|
something.
|