Skip to content

Minor changes throughout Best Practices for Reusable Bundles #7118

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 9 commits into from
38 changes: 19 additions & 19 deletions bundles/best_practices.rst
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,11 @@ the guides.
Bundle Name
-----------

A bundle is also a PHP namespace. The namespace must follow the `PSR-0`_ or
`PSR-4`_ interoperability standards for PHP namespaces and class names: it starts
with a vendor segment, followed by zero or more category segments, and it ends
with the namespace short name, which must end with a ``Bundle`` suffix.
A bundle name is also a PHP namespace. The namespace must follow the `PSR-0`_
Copy link
Member

@wouterj wouterj Nov 6, 2016

Choose a reason for hiding this comment

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

I'm against this change, the bundle is a PHP namespace. The name of that namespace just follows the bundle 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 think my reasoning was twofold: (1) the section is called "Bundle Name"; (2) the bundle contains/has a namespace whereas the namespace isn't the bundle. I was confused when reading it (being new to bundles and semi-new to namespaces) as to how the bundle became a namespace. Would it make more sense to do the following: The bundle name is used as the PHP namespace.

or `PSR-4`_ interoperability standards for PHP namespaces and class names.
The name starts with the vendor name, followed by zero or more category
segments/names, and it ends with the namespace short name which must end
with ``Bundle``.

A namespace becomes a bundle as soon as you add a bundle class to it. The
bundle class name must follow these simple rules:
Expand Down Expand Up @@ -75,7 +76,7 @@ configuration options (see below for some usage examples).
Directory Structure
-------------------

The basic directory structure of an AcmeBlogBundle must read as follows:
The basic directory structure of an ``AcmeBlogBundle`` must read as follows:
Copy link
Member

Choose a reason for hiding this comment

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

I am against this change. We usually do not enclose bundle names with backticks.

Copy link
Member

Choose a reason for hiding this comment

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

I agree with @xabbuh here. It's a name and not a code-segment. Plus, any changes in text rendering is a minor break of the read flow, so having less in-line code blocks will improve docs readability.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we then remove the backticks from the previous section? There are 3 locations where the bundle name is in backticks. ~25% of the references to AcmeBlogBundle on this page are not in backticks while the rest are. (Again, just going for consistency, but don't really care.)


.. code-block:: text

Expand Down Expand Up @@ -105,15 +106,15 @@ that automated tools can rely on:
bundles are published under the MIT license, but you can `choose any license`_;
* ``Resources/doc/index.rst``: The root file for the Bundle documentation.

The depth of sub-directories should be kept to the minimum for most used
classes and files (two levels maximum).
The depth of subdirectories should be kept to a minimum for the most used
classes and files. Two levels is the maximum.

The bundle directory is read-only. If you need to write temporary files, store
them under the ``cache/`` or ``log/`` directory of the host application. Tools
can generate files in the bundle directory structure, but only if the generated
files are going to be part of the repository.

The following classes and files have specific emplacements (some are mandatory
The following classes and files have specific emplacements (some are mandatory
Copy link
Member

Choose a reason for hiding this comment

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

This change should be reverted.

and others are just conventions followed by most developers):

=============================== ============================= ================
Expand All @@ -138,9 +139,9 @@ Classes
-------

The bundle directory structure is used as the namespace hierarchy. For
instance, a ``ContentController`` controller is stored in
``Acme/BlogBundle/Controller/ContentController.php`` and the fully qualified
class name is ``Acme\BlogBundle\Controller\ContentController``.
instance, a ``ContentController`` controller which is stored in
``Acme/BlogBundle/Controller/ContentController.php`` would have the fully
qualified class name of ``Acme\BlogBundle\Controller\ContentController``.

All classes and files must follow the :doc:`Symfony coding standards </contributing/code/standards>`.

Expand All @@ -158,8 +159,8 @@ Vendors
A bundle must not embed third-party PHP libraries. It should rely on the
standard Symfony autoloading instead.

A bundle should not embed third-party libraries written in JavaScript, CSS or
any other language.
A bundle should also not embed third-party libraries written in JavaScript,
CSS or any other language.

Tests
-----
Expand All @@ -185,7 +186,7 @@ All classes and functions must come with full PHPDoc.

Extensive documentation should also be provided in the
:doc:`reStructuredText </contributing/documentation/format>` format, under
the ``Resources/doc/`` directory; the ``Resources/doc/index.rst`` file is
the ``Resources/doc/`` directory. The ``Resources/doc/index.rst`` file is
Copy link
Contributor

Choose a reason for hiding this comment

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

does it really needs to be RST?

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. It can in in any readable format. Using rST and putting it in Resources/doc/ is necessary only when rendering the doc on symfony.com

Most bundles are using markdown, as their rendered doc is actually the github UI

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@OskarStark @stof I've reworded this section. Does this help?

the only mandatory file and must be the entry point for the documentation.

Installation Instructions
Expand Down Expand Up @@ -232,7 +233,6 @@ following standardized instructions in your ``README.md`` file.
{
$bundles = array(
// ...

new <vendor>\<bundle-name>\<bundle-long-name>(),
);

Expand Down Expand Up @@ -391,8 +391,8 @@ The ``composer.json`` file should include at least the following metadata:
``name``
Consists of the vendor and the short bundle name. If you are releasing the
bundle on your own instead of on behalf of a company, use your personal name
(e.g. ``johnsmith/blog-bundle``). The bundle short name excludes the vendor
name and separates each word with an hyphen. For example: ``AcmeBlogBundle``
(e.g. ``johnsmith/blog-bundle``). Exclude the vendor name from the bundle
short name and separate each word with an hyphen. For example: ``AcmeBlogBundle``
is transformed into ``blog-bundle`` and ``AcmeSocialConnectBundle`` is
transformed into ``social-connect-bundle``.

Expand All @@ -404,10 +404,10 @@ The ``composer.json`` file should include at least the following metadata:

``license``
``MIT`` is the preferred license for Symfony bundles, but you can use any
other license.
license.
Copy link
Contributor

@OskarStark OskarStark Nov 15, 2016

Choose a reason for hiding this comment

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

Maybe we could add a link here. I really like https://tldrlegal.com

https://tldrlegal.com/license/mit-license

Copy link
Member

Choose a reason for hiding this comment

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

Or http://choosealicense.com/ (which is run by GitHub)

Copy link
Contributor

Choose a reason for hiding this comment

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

👍


``autoload``
This information is used by Symfony to load the classes of the bundle. The
This information is used by Symfony to load the classes in the bundle. The
Copy link
Member

Choose a reason for hiding this comment

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

This change looks wrong to me. Autoloading is about how classes provided by the bundle are loaded. So we are actually talking about "classes of the bundle".

`PSR-4`_ autoload standard is recommended for modern bundles, but `PSR-0`_
standard is also supported.

Expand Down