Skip to content
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

added a maintenance document #6412

Merged
merged 1 commit into from
May 27, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions contributing/code/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ Contributing Code

bugs
patches
maintenance
core_team
security
tests
Expand Down
80 changes: 80 additions & 0 deletions contributing/code/maintenance.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
Maintenance
===========

During the lifetime of a minor version, new releases (patch versions) are
published on a monthly basis. This document describes the boundaries of
acceptable changes.

Copy link
Member

Choose a reason for hiding this comment

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

I suggest adding a general statement like:
This policy has one main goal: allow Symfony to keep getting better over time while avoiding the risk of breaking an existing and working application.

Copy link
Member 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 it adds much value. It also conveys the idea that as long as risk is low, new features could be added.

**Bug fixes** are accepted under the following conditions:
Copy link
Contributor

Choose a reason for hiding this comment

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

This may be unclear for a newbie. When someone add a PR, what makes it a bug fix? Does Bug fixes have a special meaning in the context of Symfony? Is it mandatory that a bug fix refer to an existing ticket?

Copy link
Member

Choose a reason for hiding this comment

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

@alexislefebvre bug fixes can be merged in patch versions. This means that now 3.0.0 was released, the 3.0 branch can only have bug/security fixes. New features must only be merged in the unstable branch (master/3.1 at the moment).


* The change does not break valid unit tests;
* New unit tests cover the bug fix;
* The current buggy behavior is not widely used as a "feature".

.. note::

When documentation (or phpdoc) is not in sync with the code, code behavior
should always be considered as being the correct one.

Besides bug fixes, other minor changes can be accepted in a patch version:

* **Performance improvement**: Performance improvement should only be accepted
if the changes are local (located in one class) and only for algorithmic
issues (any such patches must come with numbers that show a significant
improvement on real-world code);

* **Newer versions of PHP/HHVM**: Fixes that add support for newer versions of
PHP or HHVM are acceptable if they don't break the unit test suite;
Copy link
Member

Choose a reason for hiding this comment

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

we should also consider supporting newest versions of main OSes as bug fixes
see e.g. symfony/symfony#18385

Copy link
Member Author

Choose a reason for hiding this comment

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

added


* **Newer versions of popular OSes**: Fixes that add support for newer versions
of popular OSes (Linux, MacOS and Windows) are acceptable if they don't break
the unit test suite;

* **Translations**: Translation updates and additions are accepted;

* **External data**: Updates for external data included in Symfony can be
updated (like ICU for instance);

* **Version updates for Composer dependencies**: Changing the minimal version
of a dependency is possible, bumping to a major one or increasing PHP
minimal version is not;

* **Coding standard and refactoring**: Coding standard fixes or code
refactoring are not recommended but can be accepted for consistency with the
existing code base, if they are not too invasive, and if merging them on
master would not lead to complex branch merging;

* **Tests**: Tests that increase the code coverage can be added.

Anything not explicitly listed above should be done on the next minor or major
version instead (aka the *master* branch). For instance, the following changes
are never accepted in a patch version:

* **New features**;

* **Backward compatibility breaks**: Note that backward compatibility breaks
can be done when fixing a security issue if it would not be possible to fix
it otherwise;
Copy link
Member

Choose a reason for hiding this comment

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

Unless I'm missing something, BC breaks are never accepted in Symfony?

Copy link
Member Author

Choose a reason for hiding this comment

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

They are accepted if there is no other way to fix a security issue


* **Support for external platforms**: Adding support for new platforms (like
Google App Engine) cannot be done in patch versions;
Copy link
Contributor

Choose a reason for hiding this comment

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

symfony/symfony#18271 would violate this as well in retrospect.

Copy link
Member

Choose a reason for hiding this comment

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

which is precisely why we create this document


* **Exception messages**: Exception messages must not be changed as some
automated systems might rely on them (even if this is not recommended);

* **Adding new Composer dependencies**;
Copy link
Contributor

Choose a reason for hiding this comment

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

So the refactoring with polyfill would not be tolerated in maintainance anymore.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, and in fact we introduced most polyfills in 2.8. The ones added in 2.3/2.7 were for security fixes and bug fixes only (which doesn't mean we were right regarding this doc :) )


* **Support for newer major versions of Composer dependencies**: Taking into
account support for newer versions of an existing dependency is not
acceptable.

* **Web design**: Changing the web design of built-in pages like exceptions,
the toolbar or the profiler is not allowed.

.. note::

This policy is designed to enable a continuous upgrade path that allows one
to move forward with newest Symfony versions in the safest way. One should
be able to move PHP versions, OS or Symfony versions almost independently.
That's the reason why supporting the latest PHP versions or OS features is
considered as bug fixes.
10 changes: 6 additions & 4 deletions contributing/code/patches.rst
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,12 @@ Choose the right Branch
Before working on a patch, you must determine on which branch you need to
work:

* ``2.3``, if you are fixing a bug for an existing feature (you may have
to choose a higher branch if the feature you are fixing was introduced
in a later version);
* ``master``, if you are adding a new feature.
* ``2.3``, if you are fixing a bug for an existing feature or want to make a
change that falls into the :doc:`list of acceptable changes in patch versions
</contributing/code/maintenance>` (you may have to choose a higher branch if
the feature you are fixing was introduced in a later version);

* ``master``, if you are adding a new feature.

.. note::

Expand Down
5 changes: 5 additions & 0 deletions contributing/community/releases.rst
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ type of the release. This maintenance is divided into:
be fixed. The end of this period is referenced as being the *end of life* of
a release.

.. note::

The :doc:`maintenance document </contributing/code/maintenance>` describes
the boundaries of acceptable changes during maintenance.

Symfony Versions
----------------

Expand Down
1 change: 1 addition & 0 deletions contributing/map.rst.inc
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* :doc:`Bugs </contributing/code/bugs>`
* :doc:`Patches </contributing/code/patches>`
* :doc:`Reviewing Issues and Patches </contributing/community/reviews>`
* :doc:`Maintenance </contributing/code/maintenance>`
* :doc:`The Core Team </contributing/code/core_team>`
* :doc:`Security </contributing/code/security>`
* :doc:`Tests </contributing/code/tests>`
Expand Down