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
Improved the wording of the "Using Symfony" section
  • Loading branch information
@webmozart
webmozart committed Jan 24, 2014
commit e11335fc3712f94600134d4964cab92881b65d3d
192 changes: 111 additions & 81 deletions contributing/code/bc.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,33 +1,42 @@
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 (BC) for all further
2.x releases. Ensuring smooth upgrades of your projects is our first priority.
However, backwards compatibility comes in many different flavors.
If you are using Symfony, we promise you backwards compatibility (BC) for all
major releases (2.x, 3.x, ...). Ensuring smooth upgrades of your projects is our
Copy link
Member

Choose a reason for hiding this comment

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

we decided to use "etc" instead of "..." in the text

first priority. However, backwards compatibility comes in many different flavors.
Copy link
Member

Choose a reason for hiding this comment

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

The docs are all written in the third person perspective (except from the Quick Tour). This document should follow that as well

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 this paragraph should be extended a little bit to mention major versions (3.x, 4.x) and the fact that these versions might break BC (mentioning semantic versioning and linking to the spec might help as well).

We should probably also make it clear that breaking BC is not the end of the world. A lot of developers associate breaking BC as what happened in the past with Symfony 1/2 or ZF 1/2 BC breaks. We should say that breaking BC can lead to simple and fast upgrade as well. I suppose that we need to define a BC break here.


.. 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 was introduced with Symfony 2.3 and does not apply to previous
versions of Symfony.

This page has two different target audiences: If you are using Symfony, it will
tell you how to make sure that you will be able to upgrade smoothly to all
future 2.x versions. If you are contributing to Symfony, this page will tell you
Copy link
Member

Choose a reason for hiding this comment

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

again, mentioning a specific 2.x version here makes this document looks like it is only for version 2 of Symfony. As we are going to mention semantic versioning before, what about mentioning minor versions here (and citing 2.x as an example?)

the rules that you need to follow to ensure smooth upgrades for our users.
Copy link
Member

Choose a reason for hiding this comment

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

what about adding links to the sections (using the section headline as the reference name)?

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 too fond of that. Links here suggest that there is external, further information about that, when in reality the user is linked to the same page.


.. note::

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


Using Symfony Code
------------------

You are using Symfony in your projects? Then stick to the guidelines in this
section in order to guarantee smooth upgrades to all future 2.x versions.

If you are using Symfony in your projects, the following guidelines will help
you to ensure smooth upgrades to all future minor releases of Symfony (such as
Copy link
Contributor

Choose a reason for hiding this comment

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

remove to -> will help you ensure

Copy link
Member

Choose a reason for hiding this comment

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

both sounds fine, I think I even prefer the current version

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah i think so too, just put it up to clarify and because google results double by the thousands when removing the to

2.5, 2.6 and so on).
Copy link
Member

Choose a reason for hiding this comment

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

As you will add definitions of what is minor version and what is a major one in the introduction paragraph (see my comments above), what about removing the 2.x lists everywhere to avoid tying this document to Symfony 2.x only.


Using Our Interfaces
~~~~~~~~~~~~~~~~~~~~

In Symfony, we distinguish between regular and API interfaces. API interfaces
are marked with an ``@api`` tag in their source code. For example::
All interfaces shipped with Symfony can be used in type hints. You can also call
any of the methods that they declare. We guarantee that we won't break code that
sticks to these rules.
Copy link
Member

Choose a reason for hiding this comment

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

Not really related to this document, but that makes me even more reluctant to add interfaces for everything (see my comments on your recent PR on validators).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why? I don't really understand.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think he is saying that is a trade off, not all interfaces have the same importance or relevance. The outer or higher level of an interface should be regarded as strongly tied to the promise, whereas the inner or lesser level interfaces should allow some flexibility on the promise.


.. note::
Copy link
Member

Choose a reason for hiding this comment

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

again, I think .. caution:: is better


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

Choose a reason for hiding this comment

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

maybe emphasis (*) "never"


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``
tag in their source code::

/**
* HttpKernelInterface handles a Request to convert it to a Response.
Expand All @@ -39,40 +48,51 @@ are marked with an ``@api`` tag in their source code. For example::
interface HttpKernelInterface
{
Copy link
Member

Choose a reason for hiding this comment

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

this will break the highlighting, please do something like:

interface HttpKernelInterface
{
    // ...
}

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


In simple words, the difference between regular and API interfaces is that you
can implement API interfaces yourself and we will guarantee full backwards
compatibility. The same is not true for regular interfaces: We may, for example,
add an optional parameter or a new method to them and thus break your own
implementation.

In detail, we guarantee full backwards compatibility for the following use
cases:

============================================== ============== ==============
Use Case Regular API
============================================== ============== ==============
Type hint against Yes Yes
Call method Yes Yes
**In Implementing Classes**
Implement method No [1]_ Yes
Add custom method No [1]_ Yes
Add custom method parameter No [1]_ Yes
Add parameter default value Yes Yes
============================================== ============== ==============
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
Copy link
Member

Choose a reason for hiding this comment

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

should never is a bit strong. Users can use them but they will have to upgrade their code when upgrading. Most of the time, that's not a big deal and easy enough to do.

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.

The following table explains in detail which use cases are covered by 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.

maybe put this table in a (sub) section "Summarize"?

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 don't think having a section ("Using or interface") with text and then just one subsection makes sense.

| Use Case | Regular | API |
+-----------------------------------------------+---------------+---------------+
| If you... | Then we guarantee BC... |
+===============================================+===============+===============+
| Type hint against the interface | Yes | Yes |
+-----------------------------------------------+---------------+---------------+
| Call a method | Yes | Yes |
+-----------------------------------------------+---------------+---------------+
| **If you implement the interface and...** | **Then we guarantee BC...** |
+-----------------------------------------------+---------------+---------------+
| Implement a method | No [1]_ | Yes |
+-----------------------------------------------+---------------+---------------+
| Add a parameter to an implemented method | No [1]_ | Yes |
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 get this one. Let's say you have this interface:

interface Foo
{
  function bar($a);
}

Here is my implementation:

class ConcreteFoo implements Foo
{
  function bar($a, $b = 'a')
  {
    // here, I've added $b
  }
}

Adding $b is what you reference as "adding a parameter to an implemented method "? If yes, parameter should be argument (at least, that's how I reference such variables everywhere in Symfony). But anyway, I fail to see how this is useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's what I mean. I will change "parameter" to "argument" everywhere.

This is very useful if you want to create a class that satisfies an interface, but wants to expose additional configuration options in that method. For a simple example:

interface ValueBagInterface
{
    public function get($name);
}

class MyValueBag implements ValueBagInterface
{
    public function get($key, $default = null)
    {
        // ...
    }
}

My class can be used anywhere where that interface is expected, but when using the class directly in my code, I can take advantage of the additional argument.

+-----------------------------------------------+---------------+---------------+
| Add a default value to a parameter | Yes | Yes |
Copy link
Member

Choose a reason for hiding this comment

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

This is not really a promise that we do, but rather a promise from PHP, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. I think it should be included for sake of clarity though.

+-----------------------------------------------+---------------+---------------+

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

If you need to do any of the things marked with "No" above, feel free to
ask us whether the ``@api`` tag can be added on the respective Symfony code.
For that, simply open a `new ticket on GitHub`_.

Interfaces or interface methods tagged with ``@internal`` should never be
implemented or used.

If you think that one of our regular interfaces should have an ``@api`` tag,
put your request into a `new ticket on GitHub`_. We will then evaluate
whether we can add the tag or not.

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.

Using Our Classes
~~~~~~~~~~~~~~~~~

All classes provided by Symfony may be instantiated and accessed through their
public methods and properties.

.. note::
Copy link
Member

Choose a reason for hiding this comment

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

same comments to this directive as the previous one (both caution and emphasis)


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 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 @@ -87,57 +107,69 @@ Like API interfaces, API classes are marked with an ``@api`` tag::
{
Copy link
Member

Choose a reason for hiding this comment

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

same here (missing closing curly bracket)

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


The difference between regular and API classes is that we guarantee full
backwards compatibility if you extend an API class and override its methods,
but not for regular classes. In regular classes, we may for example add an
optional parameter to a method and thus break your overridden method.

Again, here are the details about which use cases we guarantee full backwards
compatibility for:

============================================== ============== ==============
Use Case Regular API
============================================== ============== ==============
Type hint against Yes Yes
Create instance Yes Yes
Extend Yes Yes
Access public property Yes Yes
Call public method Yes Yes
**In Extending Classes**
Access protected property No [1]_ Yes
Call protected method No [1]_ Yes
Override public property Yes Yes
Override protected property No [1]_ Yes
Override public method No [1]_ Yes
Override protected method No [1]_ Yes
Add custom property No No
Add custom method No No
Add custom method parameter No [1]_ Yes
Add parameter default value Yes Yes
============================================== ============== ==============

.. note::

If you need to do any of the things marked with "No" above, feel free to
ask us whether the ``@api`` tag can be added on the respective Symfony code.
For that, simply open a `new ticket on GitHub`_.
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
your overridden method won't match anymore and generate a fatal error.

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
"API" above), but not for the rest of the class.
"API" below), but not for the rest of the class.

To be on the safe side, check the following table to know which use cases are
covered by our backwards compatibility promise:

+-----------------------------------------------+---------------+---------------+
| Use Case | Regular | API |
+-----------------------------------------------+---------------+---------------+
| If you... | Then we guarantee BC... |
+===============================================+===============+===============+
| Type hint against the class | Yes | Yes |
+-----------------------------------------------+---------------+---------------+
| Create a new instance | Yes | Yes |
+-----------------------------------------------+---------------+---------------+
| Extend the class | Yes | Yes |
+-----------------------------------------------+---------------+---------------+
| Access a public property | Yes | Yes |
+-----------------------------------------------+---------------+---------------+
| Call a public method | Yes | Yes |
+-----------------------------------------------+---------------+---------------+
| **If you extend the class and...** | **Then we guarantee BC...** |
+-----------------------------------------------+---------------+---------------+
| Access a protected property | No [1]_ | Yes |
+-----------------------------------------------+---------------+---------------+
| Call a protected methods | No [1]_ | Yes |
+-----------------------------------------------+---------------+---------------+
| Override a public property | Yes | Yes |
+-----------------------------------------------+---------------+---------------+
| Override a protected property | No [1]_ | Yes |
+-----------------------------------------------+---------------+---------------+
| Override a public method | No [1]_ | Yes |
+-----------------------------------------------+---------------+---------------+
| Override a protected method | No [1]_ | Yes |
+-----------------------------------------------+---------------+---------------+
| Add a new property | No | No |
+-----------------------------------------------+---------------+---------------+
| Add a new method | No | No |
+-----------------------------------------------+---------------+---------------+
| Add a parameter to an overridden method | No [1]_ | Yes |
+-----------------------------------------------+---------------+---------------+
| Add a default value to a parameter | Yes | Yes |
Copy link
Member

Choose a reason for hiding this comment

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

f I understand things correctly, this should be argument here (see my comment above).

+-----------------------------------------------+---------------+---------------+

Classes, properties and methods tagged with ``@internal`` should never be
created, extended or called directly. The same applies to all classes located in
the various ``*\\Tests\\`` namespaces.
.. note::
Copy link
Member

Choose a reason for hiding this comment

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

this is a duplicate note, please create a contributing/code/includes/_api_tagging.rst.inc file with this note and include it in both places


If you think that one of our regular classes should have an ``@api`` tag,
put your request into a `new ticket on GitHub`_. We will then evaluate
whether we can add the tag or not.

Working on Symfony Code
-----------------------

Do you want to help us improve Symfony? That's great! However, please stick
to the rules listed below in order to ensure smooth upgrades for our users.


Changing Interfaces
~~~~~~~~~~~~~~~~~~~

Expand Down Expand Up @@ -166,7 +198,6 @@ Change parameter type Yes [2]_ [4]_ No
Change return type Yes [2]_ [5]_ No
============================================== ============== ==============


Changing Classes
~~~~~~~~~~~~~~~~

Expand Down Expand Up @@ -226,7 +257,6 @@ Change parameter type Yes [2]_ [4]_ No
Change return type Yes [2]_ [5]_ No
================================================== ============== ==============


.. [1] Your code may be broken by changes in the Symfony code. Such changes will
however be documented in the UPGRADE file.

Expand Down
0