Skip to content

added docs for the core team #3777

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

Closed
wants to merge 1 commit into from
Closed
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
149 changes: 149 additions & 0 deletions contributing/code/core_team.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
Symfony Core Team
=================

This document states the rules that govern the Symfony Core group. These rules
are effective upon publication of this document and all Symfony Core members
must adhere to said rules and protocol.

Core Organization
-----------------

Symfony Core members are divided into three groups. Each member can only belong
to one group at a time. The privileges granted to a group are automatically
granted to all higher priority groups.

The Symfony Core groups, in descending order of priority, are as follows:

1. **Project Leader**

* Elects members in any other group;
* Merges pull requests in all Symfony repositories.

2. **Mergers**

* Merge pull requests for the component or components on which they have been
Copy link
Member

Choose a reason for hiding this comment

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

Merges pull requests [...] to be consistent

Copy link
Member Author

Choose a reason for hiding this comment

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

No, there is one project leaders but many mergers.

granted privileges.

3. **Deciders**

* Decide to merge or reject a pull request.
Copy link
Member

Choose a reason for hiding this comment

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

Decides

Copy link
Member Author

Choose a reason for hiding this comment

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

same as above


Active Core Members
~~~~~~~~~~~~~~~~~~~

.. role:: leader
.. role:: merger
.. role:: decider

* **Project Leader**:

* **Fabien Potencier** (:leader:`fabpot`).

* **Mergers**:

* **Bernhard Schussek** (:merger:`webmozart`) can merge into the Form_,
Validator_, Icu_, Intl_, Locale_, OptionsResolver_ and PropertyAccess_
components;

* **Tobias Schultze** (:merger:`Tobion`) can merge into the Routing_
component;

* **Romain Neutron** (:merger:`romainneutron`) can merge into the
Process_ component;

* **Nicolas Grekas** (:merger:`nicolas-grekas`) can merge into the Debug_
component.

* **Deciders**:

* **Christophe Coevoet** (:decider:`stof`);
* **Jakub Zalas** (:decider:`jakzal`);
* **Jordi Boggiano** (:decider:`seldaek`);
* **Lukas Kahwe Smith** (:decider:`lsmith77`).

Core Membership Application
~~~~~~~~~~~~~~~~~~~~~~~~~~~

At present, new Symfony Core membership applications are not accepted.

Core Membership Revocation
~~~~~~~~~~~~~~~~~~~~~~~~~~

A Symfony Core membership can be revoked for any of the following reasons:

* Refusal to follow the rules and policies stated in this document;
* Lack of activity for the past six months;
* Willful negligence or intent to harm the Symfony project;
* Upon decision of the **Project Leader**.

Should new Symfony Core memberships be accepted in the future, revoked
members must wait at least 12 months before re-applying.

Code Development Rules
----------------------

Symfony project development is based on pull requests proposed by any member
of the Symfony community. Pull request acceptance or rejection is decided based
on the votes cast by the Symfony Core members.
Copy link
Member

Choose a reason for hiding this comment

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

-1 for this, it's open source. The opinion of other, non core, contributors should also be considered when accepting/rejecting a PR imo

Copy link
Member

Choose a reason for hiding this comment

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

If you think about it, the new workflow greatly improves the current situation. In the past, when the discussion about a pull request reached and endpoint, it was only Fabien who decided what to do about that pull request.

With this new workflow, a lot of different people can decide what to do. And most of the Symfony Core members aren't SensioLabs employees, so I think that Fabien is acknowledging the importance of the community.

Copy link
Member

Choose a reason for hiding this comment

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

I can see that this policy reduces the power of Fabian :)

But it should be reworded a bit. In the current state, it implies that only the opinions of core members counts. That can give "normal" contributors a feeling that their opinion isn't important.

Copy link
Member Author

Choose a reason for hiding this comment

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

Discussions are more than encouraged, and they certainly influence votes from the deciders. But at the end of the day, and this has nothing to do with being Open-Source or not, someone has to decide whether a feature should be included or not; and that's the role of the core team.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 i see this as a great advancement in the community, i mean this document. Thank you for taking the time to do this really.

Copy link
Contributor

Choose a reason for hiding this comment

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

@wouterj the assumption is of course that just as before, the opinions of the community are taking into advisement with all decisions. Now it can of course be noted as a fact in the process but imho that complicates the definition of the actual process for a "political" gain. Not saying that politics isn't important and politics is a natural part of anything that involves more than a few people (for example the process could be used by some people to bad mouth Symfony saying "look they do not care about their community opinions). But in the grand scheme of things what will matter the most is the actions in practice.


Pull Request Voting Policy
~~~~~~~~~~~~~~~~~~~~~~~~~~

* ``-1`` votes must always be justified by technical and objective reasons;

* ``+1`` votes do not require justification, unless there is at least one
``-1`` vote;

* Core members can change their votes as many times as they desire
during the course of a pull request discussion;

* Core members are not allowed to vote on their own pull requests.

Pull Request Merging Policy
~~~~~~~~~~~~~~~~~~~~~~~~~~~

A pull request **can be merged** if:

* Enough time was given for peer reviews (a few minutes for typos or minor
changes, at least 2 days for "regular" pull requests, and 4 days for pull
requests with "a significant impact");

* It is a minor change [1]_, regardless of the number of votes;

* At least the component's **Merger** or two other Core members voted ``+1``
and no Core member voted ``-1``.
Copy link
Member

Choose a reason for hiding this comment

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

so if one core member votes -1, the PR is not merged?

Copy link
Member

Choose a reason for hiding this comment

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

@wouterj that's the idea. But keep in mind that "deciders" and other core members must always provide technical and objective reasons for their votes, so we don't allow mere opinions.

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 have thought two would be safer for a complete rejection as whilst those reasons could be valid, all the other core team members would take those reasons into account but they can still make concious decisions as to whether they agree those reasons are enough to stop the progression of the PR.

This is a problem with the PHP core and that's that because it requires a 2/3 majority, major things rarely get through as they'll get over 50% but not over 66% as there are valid technical reasons for a no vote, it's just others view the weighting/importance of those reasons to be different.

Also, is there no clause for the Project Leader to overrule the core team on this?

Copy link
Member

Choose a reason for hiding this comment

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

@wouterj the idea is that when a core member says -1, it means that the PR needs to be changed so that this guy changes his vote, or that the other contributors need to convince him that the current PR is the right implementation.
Keep in mind that a -1 does not mean that the PR gets rejected immediately. It means that it requires more work before having a second chance (well, in some cases it will mean a rejection depending of the reasons of the vote). And also keep in mind that a -1 vote can be changed later when the discussion on the PR continues

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 for 2.x we should rather be conservative, so it makes sense to have one -1 stop things. For 3.x it might be a bit trickier, but it essentially means that if someone is -1 we would need to come to a consensus somehow. It requires people to be reasonable so that in the end it will not be in-politics and development by compromise.


Pull Request Merging Process
~~~~~~~~~~~~~~~~~~~~~~~~~~~~

All code must be committed to the repository through pull requests, except for
minor changes [1]_ which can be committed directly to the repository.

**Mergers** must always use the command-line ``gh`` tool provided by the
**Project Leader** to merge the pull requests.

Release Policy
~~~~~~~~~~~~~~

The **Project Leader** is also the release manager for every Symfony version.

Symfony Core Rules and Protocol Amendments
------------------------------------------

The rules described in this document may be amended at anytime at the
discretion of the **Project Leader**.


Copy link
Member

Choose a reason for hiding this comment

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

There's no need for two blank lines, is there?

.. [1] Minor changes comprise typos, DocBlock fixes, code standards
violations, and minor CSS, JavaScript and HTML modifications.

.. _Form: https://github.com/symfony/Form
.. _Validator: https://github.com/symfony/Validator
.. _Icu: https://github.com/symfony/Icu
.. _Intl: https://github.com/symfony/Intl
.. _Locale: https://github.com/symfony/Locale
.. _OptionsResolver: https://github.com/symfony/OptionsResolver
.. _PropertyAccess: https://github.com/symfony/PropertyAccess
.. _Routing: https://github.com/symfony/Routing
.. _Process: https://github.com/symfony/Process
.. _Debug: https://github.com/symfony/Debug
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
core_team
security
tests
bc
Expand Down
1 change: 1 addition & 0 deletions contributing/map.rst.inc
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

* :doc:`Bugs </contributing/code/bugs>`
* :doc:`Patches </contributing/code/patches>`
* :doc:`The Core Team </contributing/code/core_team>`
* :doc:`Security </contributing/code/security>`
* :doc:`Tests </contributing/code/tests>`
* :doc:`Backwards Compatibility </contributing/code/bc>`
Expand Down