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.

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
Show file tree
Hide file tree
Changes from 10 commits
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
256 changes: 256 additions & 0 deletions contributing/code/bc.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,256 @@
Our Backwards Compatibility Promise
Copy link
Member

Choose a reason for hiding this comment

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

Just use "The Backwards Compatibility Promise", we didn't speak of "our" release cycle neither

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'm not sure this feels right here. A promise is something personal, something that has only value if you know who it is from. So I think the first person and use of "our" here makes the document more credible.

===================================

As of Symfony 2.3, we promise you backwards compatibility for all further 2.x
releases. Ensuring smooth upgrades of your projects is our first priority.
However, backwards compatibility comes in many different flavors. This page
lists which code changes are covered by our promise and to what degree.

If you are contributing to Symfony, make sure that your code changes comply to
Copy link
Contributor

Choose a reason for hiding this comment

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

I would perhaps say "....comply with the rules" rather than "comply to". Either that or "adhere to".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you! Language suggestions are more than welcome.

the rules listed below.

.. note::
Copy link
Member

Choose a reason for hiding this comment

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

I think a .. caution:: better fits the content


This promise is in trial until April 15th, 2014. Until then, we may change
parts of it if we discover problems or limitations.
Copy link
Member

Choose a reason for hiding this comment

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

This should be removed. As with any other document, it can be amended if needed.



Interfaces
----------

Normal Interfaces
~~~~~~~~~~~~~~~~~

All interfaces in the ``Symfony`` namespace are **safe for use**. That means
that:

* You can safely type hint against the interface.

* You can safely use any of the methods provided by the interface.

However:

* You cannot safely implement the interface. The interface may change, but all
changes will be documented in the UPGRADE file.

Methods tagged with ``@api`` are treated as if they belonged to an API
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() { ... }
}

API Interfaces
~~~~~~~~~~~~~~

All interfaces tagged with ``@api`` are also **safe for implementation**. That
means that:

* You can safely implement the interface.


Copy link
Member

Choose a reason for hiding this comment

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

Small formatting nitpick: you have a bunch of double-newlines which should be converted to single-newlines.

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 left them in before headings. I'll remove them however.

Safe Operations
~~~~~~~~~~~~~~~

The following table summarizes the safe operations when using our interfaces:

============================================== ============== ==============
Operation Normal API
============================================== ============== ==============
Type hint against Safe Safe
Use method Safe Safe
**When Implementing**
Implement method Not Safe Safe
Add custom method Not Safe Not Safe
Add custom method parameter Not Safe Not Safe
Add parameter default value Safe Safe
============================================== ============== ==============


Allowed Changes
~~~~~~~~~~~~~~~

This table tells you which changes you are allowed to do when working on the
code of Symfony's interfaces:

============================================== ============== ==============
Copy link
Contributor

Choose a reason for hiding this comment

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

wonder why you changed the format of the table from above to this other format

Copy link
Member

Choose a reason for hiding this comment

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

It's easier to maintain if the table does not have new line feeds. But we should change the other new tables, to be consistent

Copy link
Contributor

Choose a reason for hiding this comment

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

is that a new adoption on tables in the documentation in general too? interesting i did not know

Copy link
Member

Choose a reason for hiding this comment

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

I don't know. We just choose per table which format is best (I think most of them uses the +--+ format)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was the other way around. I used this format for the above tables initially. But then I introduced cells which span multiple columns above, so I had to switch to the +--+ format.

Type of Change Normal API
============================================== ============== ==============
Remove entirely No No
Change name or namespace No No
Add parent interface Yes [1]_ No
Remove parent interface No No
**Methods**
Add method Yes [1]_ No
Remove method No No
Change name No No
Add parameter without a default value No No
Add parameter with a default value Yes [1]_ No
Remove parameter Yes [2]_ Yes [2]_
Add default value to a parameter Yes [1]_ No
Remove default value of a parameter No No
Add type hint to a parameter No No
Remove type hint of a parameter Yes [1]_ No
Change return type Yes [1]_ [3]_ No
============================================== ============== ==============


Classes
-------

Normal Classes
~~~~~~~~~~~~~~

All classes in the ``Symfony`` namespace are **safe for use**. That means that:

* You can safely type hint against the class' name.

* You can safely create new instances.

* You can safely extend the class.

* You can safely use public properties and methods.

When extending the class:

* You can safely override public properties.
Copy link

Choose a reason for hiding this comment

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

But not public methods?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only if the class is tagged @api. The reason is that we may change the function signature in a call-safe way (for example, add an optional parameter) that doesn't break calling code, but breaks overriding classes.


However:

* You cannot safely override methods in extending classes. The class may change,
but all changes will be documented in the UPGRADE file.

Properties and methods tagged with ``@api`` are treated as if they belonged
to an API class.


API Classes
~~~~~~~~~~~

All classes tagged with ``@api`` are also **safe for extension**. That means
that:

* You can safely use protected properties and methods.

* You can safely override protected properties.

* You can safely override public or protected methods.


Safe Operations
~~~~~~~~~~~~~~~

The following table summarizes the safe operations when using our classes:

============================================== ============== ==============
Operation Normal API
============================================== ============== ==============
Type hint against Safe Safe
Create instance Safe Safe
Extend Safe Safe
Use public property Safe Safe
Use public method Safe Safe
**When Extending**
Use protected property Not Safe Safe
Use protected method Not Safe Safe
Override public property Safe Safe
Override protected property Not Safe Safe
Override public method Not Safe Safe
Override protected method Not Safe Safe
Add custom method Not Safe Not Safe
Add custom method parameter Not Safe Not Safe
Add parameter default value Safe Safe
============================================== ============== ==============


Allowed Changes
~~~~~~~~~~~~~~~

This table tells you which changes you are allowed to do when working on the
code of Symfony's classes

================================================== ============== ==============
Type of Change Normal API
================================================== ============== ==============
Remove entirely No No
Make final Yes [1]_ No
Make abstract No No
Change name or namespace No No
Change parent class Yes [4]_ Yes [4]_
Add interface Yes Yes
Remove interface No No
**Public Properties**
Add public property Yes Yes
Remove public property No No
Reduce visibility No No
**Protected Properties**
Add protected property Yes Yes
Remove protected property Yes [1]_ No
Reduce visibility Yes [1]_ No
**Constructors**
Add constructor without mandatory parameters Yes [1]_ Yes [1]_
Remove constructor Yes [1]_ No
Reduce visibility of a public constructor No No
Reduce visibility of a protected constructor Yes [1]_ No
**Public Methods**
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 [1]_ No
Remove parameter Yes [2]_ Yes [2]_
Add default value to a parameter Yes [1]_ No
Remove default value of a parameter No No
Add type hint to a parameter Yes [5]_ No
Remove type hint of a parameter Yes [1]_ No
Change return type Yes [1]_ [3]_ No
**Protected Methods**
Add protected method Yes Yes
Remove protected method Yes [1]_ No
Change name No No
Reduce visibility Yes [1]_ No
Add parameter without a default value Yes [1]_ No
Add parameter with a default value Yes [1]_ No
Remove parameter Yes [2]_ Yes [2]_
Add default value to a parameter Yes [1]_ No
Remove default value of a parameter Yes [1]_ No
Add type hint to a parameter Yes [1]_ No
Remove type hint of a parameter Yes [1]_ No
Change return type Yes [1]_ [3]_ No
================================================== ============== ==============


.. [1] Should be avoided. When done, this change must be documented in the
UGPRADE file.
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: UPGRADE (apologies, I added this comment previously on an outdated diff)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed


.. [2] Only the last parameter(s) of a method may be removed.

.. [3] The return type may only be changed to compatible types. The following
type changes are allowed:

=================== ==================================================================
Original Type New Type
=================== ==================================================================
boolean any `scalar type`_ with equivalent `boolean values`_
string any `scalar type`_ or object with equivalent `string values`_
integer any `scalar type`_ with equivalent `integer values`_
float any `scalar type`_ with equivalent `float values`_
array instance of ``ArrayAccess``, ``Traversable`` and ``Countable``
``ArrayAccess`` array
``Traversable`` array
``Countable`` array
=================== ==================================================================

.. [4] When changing the parent class, the original parent class must remain an
ancestor of the class.

.. [5] A type hint may only be added if passing a value with a different type
previously generated a fatal error.

.. _scalar type: http://php.net/manual/en/function.is-scalar.php

.. _boolean values: http://php.net/manual/en/function.boolval.php

.. _string values: http://www.php.net/manual/en/function.strval.php

.. _integer values: http://www.php.net/manual/en/function.intval.php

.. _float values: http://www.php.net/manual/en/function.floatval.php
1 change: 1 addition & 0 deletions contributing/code/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ Contributing Code
patches
security
tests
bc
standards
conventions
git
Expand Down
3 changes: 2 additions & 1 deletion contributing/map.rst.inc
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@
* :doc:`Patches </contributing/code/patches>`
* :doc:`Security </contributing/code/security>`
* :doc:`Tests </contributing/code/tests>`
* :doc:`Backwards Compatibility </contributing/code/bc>`
* :doc:`Coding Standards</contributing/code/standards>`
* :doc:`Code Conventions</contributing/code/conventions>`
* :doc:`Git</contributing/code/git>`
* :doc:`Git</contributing/code/git>`
* :doc:`License </contributing/code/license>`

* **Documentation**
Expand Down