8000 [Review] Added detailed Backwards Compatibility Promise text by webmozart · Pull Request #3439 · symfony/symfony-docs · GitHub
[go: up one dir, main page]
Skip to content

[Review] Added detailed Backwards Compatibility Promise text #3439

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 39 commits into from
Feb 20, 2014
Merged

[Review] Added detailed Backwards Compatibility Promise text #3439

Changes from 1 commit
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
840073c
Added detailed BC promise text
webmozart Jan 7, 2014
7320ed0
Updated BC promise to be valid as of Symfony 2.3
webmozart Jan 7, 2014
dacd7ce
Rearranged rules to be more easily understandable
webmozart Jan 7, 2014
79ca9f7
Added information about type compatibility
webmozart Jan 8, 2014
0e925cb
Added tables with safe operations
webmozart Jan 8, 2014
44ecf16
Fixed: No parameters must be added (ever) to API class methods
webmozart Jan 8, 2014
afadaab
Changed: The last parameters of a method may be removed
webmozart Jan 8, 2014
345410c
Rearranged safe operations to make more sense
webmozart Jan 8, 2014
a3ad08c
Removed most of the "cannot" statements which are repeated in the tab…
webmozart Jan 8, 2014
31ab2db
Improved wording
webmozart Jan 8, 2014
502ed95
Added: Some breaking changes to unsafe operations are documented in t…
webmozart Jan 8, 2014
4c5a55d
Rearranged page to have different sections for different user bases
webmozart Jan 8, 2014
c6e850d
Language fixes
webmozart Jan 8, 2014
db76288
Fixed headings
webmozart Jan 8, 2014
54fd836
Language improvements
webmozart Jan 8, 2014
00c6ebe
Fixed safety statements
webmozart Jan 8, 2014
efd3911
Added that adding custom properties is not safe
webmozart Jan 8, 2014
dcbe79a
Improved wording
webmozart Jan 9, 2014
af3a645
Added note about requesting `@api` tags
webmozart Jan 9, 2014
be76644
Added information about internal classes and interfaces
webmozart Jan 9, 2014
dfb3e8b
Improved wording
webmozart Jan 9, 2014
6501a35
Added information about changing return types that are classes or int…
webmozart Jan 9, 2014
0c6420f
Added information about changing parameter types
webmozart Jan 9, 2014
69768dd
Improved wording: use -> call, access
webmozart Jan 10, 2014
5a160c5
Added note about deprecated interfaces/classes
webmozart Jan 10, 2014
ef1f021
Added note about test classes
webmozart Jan 10, 2014
6d9edf1
Improved wording: Changed "safe" to "guaranteed"
webmozart Jan 23, 2014
8c6c7bf
Simplified usage description
webmozart Jan 23, 2014
4868452
Added prose about the difference between regular/API classes/interfaces
webmozart Jan 23, 2014
e11335f
Improved the wording of the "Using Symfony" section
webmozart Jan 24, 2014
25443c0
Improved table formatting
webmozart Feb 12, 2014
11bb879
Grammar
webmozart Feb 12, 2014
fd1d912
Typo
webmozart Feb 12, 2014
bdd3c03
Implemented changes suggested by @WouterJ
webmozart Feb 15, 2014
2320906
Extracted duplicated text into _api_tagging.rst.inc
webmozart Feb 15, 2014
90c4de6
Mentioned Semantic Versioning in the introduction
webmozart Feb 20, 2014
be2251c
Implemented @fabpot's comments
webmozart Feb 20, 2014
ce58ee9
Added rules for adding parent interfaces and moving methods/propertie…
webmozart Feb 20, 2014
0717192
Removed useless line break
webmozart Feb 20, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Implemented @fabpot's comments
  • Loading branch information
@webmozart
webmozart committed Feb 20, 2014
commit be2251c877ae2c5778afa66d77f2241f193ab7cf
108 changes: 71 additions & 37 deletions contributing/code/bc.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ sticks to these rules.
.. caution::

The exception to this rule are interfaces tagged with ``@internal``. Such
interfaces should *never* be used or implemented.
interfaces should not be used or implemented.

If you want to implement an interface, you should first make sure that the
interface is an API interface. You can recognize API interfaces by the ``@api``
Expand All @@ -69,9 +69,15 @@ tag in their source code::
}

If you implement an API interface, we promise that we won't ever break your
code. Regular interfaces, by contrast, should never be implemented, because if
we add a new method to the interface or add a new optional parameter to the
signature of a method, we would generate a fatal error in your application.
code. Regular interfaces, by contrast, may be extended between minor releases,
for example by adding a new method. Be prepared to upgrade your code manually
if you implement a regular interface.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The meaning of Yes and No are hard to understand for me. Actually, it took me quite some time to figure out what the whole table actually means.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For instance, what does Add custom method mean? Is it when I add a custom method on a class that implements a Symfony interface? Or does it mean that we are allowed to add a new method in a Symfony interface?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole section is targeted at Symfony users (not developers; the dev section is below). So "Add custom method" means that a user should not add methods that do not exist in our interface (i.e. we cannot guarantee BC).

Example:

/** regular interface, not API */
interface SymfonyInterface
{
    public function foo();
}

class MyImplementation implements SymfonyInterface
{
    // BC guaranteed for this method
    public function foo() { ... }

    // BC not guaranteed for this method (if we add bar() to SomeInterface, but with a different signature)
    public function bar() { ... }
}

.. note::

Even if we do changes that require manual upgrades, we limit ourselves to
changes that can be upgraded easily. We will always document the precise
upgrade instructions in the UPGRADE file in Symfony's root directory.

The following table explains in detail which use cases are covered by our
backwards compatibility promise:
Expand All @@ -89,9 +95,9 @@ backwards compatibility promise:
+-----------------------------------------------+---------------+---------------+
| Implement a method | No [1]_ | Yes |
+-----------------------------------------------+---------------+---------------+
| Add a parameter to an implemented method | No [1]_ | Yes |
| Add an argument to an implemented method | No [1]_ | Yes |
+-----------------------------------------------+---------------+---------------+
| Add a default value to a parameter | Yes | Yes |
| Add a default value to an argument | Yes | Yes |
+-----------------------------------------------+---------------+---------------+

.. include:: _api_tagging.rst.inc
Expand All @@ -107,7 +113,7 @@ public methods and properties.
Classes, properties and methods that bear the tag ``@internal`` as well as
the classes located in the various ``*\\Tests\\`` namespaces are an
exception to this rule. They are meant for internal use only and should
*never* be accessed by your own code.
not be accessed by your own code.

Just like with interfaces, we also distinguish between regular and API classes.
Like API interfaces, API classes are marked with an ``@api`` tag::
Expand All @@ -127,9 +133,15 @@ Like API interfaces, API classes are marked with an ``@api`` tag::
The difference between regular and API classes is that we guarantee full
backwards compatibility if you extend an API class and override its methods. We
can't give the same promise for regular classes, because there we may, for
example, add an optional parameter to a method. Consequently, the signature of
example, add an optional argument to a method. Consequently, the signature of
your overridden method wouldn't match anymore and generate a fatal error.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we emphasize (peharps in a note) that we will only make "simple" changes even on regular classes (such as the one you mention here) and mention that it make upgrading needed but simple enough? That way, we make it clear what we are allowed to break.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Referencing the following section about Symfony contributors might be enough.


.. note::

As with interfaces, we limit ourselves to changes that can be upgraded
easily. We will document the precise ugprade instructions in the UPGRADE
file in Symfony's root directory.

In some cases, only specific properties and methods are tagged with the ``@api``
tag, even though their class is not. In these cases, we guarantee full backwards
compatibility for the tagged properties and methods (as indicated in the column
Expand Down Expand Up @@ -171,9 +183,13 @@ covered by our backwards compatibility promise:
+-----------------------------------------------+---------------+---------------+
| Add a new method | No | No |
+-----------------------------------------------+---------------+---------------+
| Add a parameter to an overridden method | No [1]_ | Yes |
| Add an argument to an overridden method | No [1]_ | Yes |
+-----------------------------------------------+---------------+---------------+
| Add a default value to an argument | Yes | Yes |
+-----------------------------------------------+---------------+---------------+
| Call a private method (via Reflection) | No | No |
+-----------------------------------------------+---------------+---------------+
| Add a default value to a parameter | Yes | Yes |
| Access a private property (via Reflection) | No | No |
+-----------------------------------------------+---------------+---------------+

.. include:: _api_tagging.rst.inc
Expand Down Expand Up @@ -201,14 +217,14 @@ Remove parent interface No No
Add method Yes [2]_ No
Remove method No No
Change name No No
Add parameter without a default value No No
Add parameter with a default value Yes [2]_ No
Remove parameter Yes [3]_ Yes [3]_
Add default value to a parameter Yes [2]_ No
Remove default value of a parameter No No
Add type hint to a parameter No No
Remove type hint of a parameter Yes [2]_ No
Change parameter type Yes [2]_ [4]_ No
Add argument without a default value No No
Add argument with a default value Yes [2]_ No
Remove argument Yes [3]_ Yes [3]_
Add default value to an argument Yes [2]_ No
Remove default value of an argument No No
Add type hint to an argument No No
Remove type hint of an argument Yes [2]_ No
Change argument type Yes [2]_ [4]_ No
Change return type Yes [2]_ [5]_ No
============================================== ============== ==============

Expand Down Expand Up @@ -236,8 +252,11 @@ Reduce visibility No No
Add protected property Yes Yes
Remove protected property Yes [2]_ No
Reduce visibility Yes [2]_ No
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should add lines about private properties and methods. My reasoning here is that you can access them from the outside via reflection. Everyone would agree that there no guarantee about those properties/methods, but as we are listing all possible cases here, it does not hurt to list all possibilities here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added notes about private properties and methods both in the "Using Symfony" and "Working on Symfony" sections.

**Private Properties**
Add private property Yes Yes
Remove private property Yes Yes
**Constructors**
Add constructor without mandatory parameters Yes [2]_ Yes [2]_
Add constructor without mandatory arguments Yes [2]_ Yes [2]_
Remove constructor Yes [2]_ No
Reduce visibility of a public constructor No No
Reduce visibility of a protected constructor Yes [2]_ No
Expand All @@ -246,29 +265,43 @@ Add public method Yes Yes
Remove public method No No
Change name No No
Reduce visibility No No
Add parameter without a default value No No
Add parameter with a default value Yes [2]_ No
Remove parameter Yes [3]_ Yes [3]_
Add default value to a parameter Yes [2]_ No
Remove default value of a parameter No No
Add type hint to a parameter Yes [7]_ No
Remove type hint of a parameter Yes [2]_ No
Change parameter type Yes [2]_ [4]_ No
Add argument without a default value No No
Add argument with a default value Yes [2]_ No
Remove argument Yes [3]_ Yes [3]_
Add defa 9E7A ult value to an argument Yes [2]_ No
Remove default value of an argument No No
Add type hint to an argument Yes [7]_ No
Remove type hint of an argument Yes [2]_ No
Change argument type Yes [2]_ [4]_ No
Change return type Yes [2]_ [5]_ No
**Protected Methods**
Add protected method Yes Yes
Remove protected method Yes [2]_ No
Change name No No
Reduce visibility Yes [2]_ No
Add parameter without a default value Yes [2]_ No
Add parameter with a default value Yes [2]_ No
Remove parameter Yes [3]_ Yes [3]_
Add default value to a parameter Yes [2]_ No
Remove default value of a parameter Yes [2]_ No
Add type hint to a parameter Yes [2]_ No
Remove type hint of a parameter Yes [2]_ No
Change parameter type Yes [2]_ [4]_ No
Add argument without a default value Yes [2]_ No
Add argument with a default value Yes [2]_ No
Remove argument Yes [3]_ Yes [3]_
Add default value to an argument Yes [2]_ No
Remove default value of an argument Yes [2]_ No
Add type hint to an argument Yes [2]_ No
Remove type hint of an argument Yes [2]_ No
Change argument type Yes [2]_ [4]_ No
Change return type Yes [2]_ [5]_ No
**Private Methods**
Add private method Yes Yes
Remove private method Yes Yes
Change name Yes Yes
Reduce visibility Yes Yes
Add argument without a default value Yes Yes
Add argument with a default value Yes Yes
Remove argument Yes Yes
Add default value to an argument Yes Yes
Remove default value of an argument Yes Yes
Add type hint to an argument Yes Yes
Remove type hint of an argument Yes Yes
Change argument type Yes Yes
Change return type Yes Yes
================================================== ============== ==============

.. [1] Your code may be broken by changes in the Symfony code. Such changes will
Expand All @@ -277,9 +310,10 @@ Change return type Yes [2]_ [5]_ No
.. [2] Should be avoided. When done, this change must be documented in the
UPGRADE file.

.. [3] Only the last parameter(s) of a method may be removed.
.. [3] Only the last argument(s) of a method may be removed, as PHP does not
care about additional arguments that you pass to a method.

.. [4] The parameter type may only be changed to a compatible or less specific
.. [4] The argument type may only be changed to a compatible or less specific
type. The following type changes are allowed:

=================== ==================================================================
Expand Down
0