Plugin naming convention RFC implementation (closes #8848) #8903

stefandoorn · 2017-10-25T15:07:10Z

Q	A
Branch?	master
Bug fix?	no
New feature?	no
BC breaks?	yes (sort of.. but no in docs)
Deprecations?	no
Related tickets	fixes #8848
License	MIT

After discussion in #8848, hereby the PR. Difference from the discussion is that I've chosen to name the repository after the class bundle name, to have it all consistent and in the same style.

Before merging this, there should also come a PR to the PluginSkeleton to rename things I think.

pamil · 2017-10-26T09:38:13Z

docs/plugins/creating-plugin.rst

@@ -64,11 +64,14 @@ to ensure your plugin's extraordinary quality.

 Besides the way you are creating plugins (based on our skeleton or on your own), there are a few naming conventions that should be followed:

- * Bundle class must have a ``Plugin`` suffix instead of ``Bundle`` in its name (e.g. InvoicePlugin).
+ * Bundle class must have a ``Sylius`` prefix, and a ``Plugin`` suffix (instead of ``Bundle``) in its name (e.g. ``SyliusInvoicePlugin``).


I'd make it vendor name + Sylius + actual name + Plugin. The reasoning behind it is to distinct official Sylius plugins from the community made ones. SyliusInvoicePlugin uses sylius_invoice as config root name, whereas AcmeSyliusInvoicePlugin uses acme_sylius_invoice.

True.. Downside is that e.g. in my case using my own name we get to something like this: StefanDoornSyliusInvoicePlugin.. sounds silly :-) The vendor is already in the namespace and the repository is in another vendor, isn't that enough to make people (developers only) understand what the source of it is?

These are only two files, bundle one and extension one, it's most likely to be an one time thing. The issue here is to have distinct config keys.

For Composer package name and Github repos it's already solved by prefixing with vendor-name/.

Ah that way. Ok, I'll check it. Alternative could be to adjust the trait to adjust the config key I think based on PSR4 namespace.

Is there really a need to distinct official Sylius plugins from the community made plugins? There should be one rule for them all IMO.
And Sylius + name + Plugin seems simple and enough.

Using vendor names in package names / configuration keys is a common practice, I can't think of Symfony bundle which does not follow this convention.

Let's just take our most common plugin, CMS one, there's one from Lakion and one from BitBag. If we stay with SyliusCmsPlugin, the configuration key will always be sylius_cms, which might cause confusion. And then FriendsOfSylius or Sylius itself wants to unify them... creating another one with the same configuration key.

I see the point. Is it possible to do this by adjusting the trait? What I'm trying is to get the name of the repo and the class name aligned, but having diversified configs keys :)

As for the currently existing trait, changing its behaviour will be a BC break and having two of them might be misleading.

Makes sense, not an option. Could you (@pamil, @pjedrzejewski, @CoderMaggie) let me know what the naming convention regarding this will be? Then I will implement accordingly.

I guess we can end up with:

Composer package name: vendor-name/sylius-something-plugin
Bundle / extension classname: VendorNameSyliusSomethingPlugin / VendorNameSyliusSomethingExtension - namespace does not matter, probably in VendorName\SyliusSomethingPlugin

pamil · 2017-10-26T09:38:39Z

docs/plugins/creating-plugin.rst

 * Bundle class must use the ``Sylius\Bundle\CoreBundle\Application\SyliusPluginTrait``.
- * The name of the extension in DependencyInjection folder must follow the regular Symfony rules (e.g. InvoiceExtension).
- * The plugin shouldn't have Sylius prefix in its name. ``Plugin`` suffix in terms of bundles is unique for Sylius at the moment.
+ * The name of the extension in the DependencyInjection folder must follow the regular Symfony rules (e.g. ``SyliusInvoiceExtension``).


Then the extension name changes accordingly.

stefandoorn · 2017-11-07T20:31:02Z

Changed a lot and added examples. Let me know what you think.

pamil

Looks awesome! Can you switch this PR's base branch to 1.0?

pamil · 2017-11-08T10:57:27Z

docs/plugins/creating-plugin.rst

+ * Bundle class name should start with vendor name, followed by ``Sylius`` and suffixed by ``Plugin`` (instead of ``Bundle``), e.g.: ``VendorNameSyliusInvoicePlugin``.
+ * Bundle extension should be named similar, but suffixed by the Symfony standard ``Extension``, e.g.: ``VendorNameSyliusInvoiceExtension``.
+ * Bundle class must use the ``Sylius\Bundle\CoreBundle\Application\SyliusPluginTrait`` trait.
+ * Namespace should follow _`PSR-4 <http://www.php-fig.org/psr/psr-4/>`. It should start with vendor name, followed by ``Sylius`` and suffixed by ``Plugin`` (e.g. ``VendorName\SyliusInvoicePlugin``)


It should start with vendor name, followed by Sylius and suffixed by Plugin (e.g. VendorName\SyliusInvoicePlugin) -> Top-level namespace should be the vendor name and the second-level one should be prefixed with...?

pamil · 2017-11-08T11:00:32Z

docs/plugins/creating-plugin.rst

-The following rules are applied to all bundles which will provide an integration with the whole Sylius platform
-(``sylius/sylius`` or ``sylius/core-bundle`` in vendors). Reusable components for the whole Symfony community, which will be based
-just on some Sylius bundles should follow regular Symfony conventions.
+    "autoload": {


The build is failing because the given JSON snippet is invalid, let's change it to:

{ "autoload": { "psr-4": { "VendorName\\SyliusInvoicePlugin\\": "src/", "Tests\\VendorName\\SyliusInvoicePlugin\\": "tests/" } } }

stefandoorn · 2017-11-08T18:02:30Z

@pamil I think we are good to go now.

pamil

Great one!

pamil · 2017-11-09T11:13:41Z

This one should be merged together with Sylius/PluginSkeleton#56.

pjedrzejewski · 2017-11-09T11:26:00Z

Thank you very much Stefan! 👍 Great work.

lsmith77 · 2017-11-09T12:28:18Z

docs/plugins/creating-plugin.rst

@@ -11,11 +11,11 @@ which has built-in infrastructure for designing and testing using `Behat`_.

 .. code-block:: bash

-    $ composer create-project sylius/plugin-skeleton MyPlugin
+    $ composer create-project sylius/plugin-skeleton SyliusMyFirstPlugin


did you intentional leave out the vendor name here?

lsmith77 · 2017-11-09T12:29:53Z

I noticed that the conventions do not see to cover DI service as well as command names. Should this maybe be covered as well?

stefandoorn mentioned this pull request Oct 25, 2017

RFC: Sylius plugin naming convention - allow Sylius in plugin name #8848

Closed

Zales0123 added the Documentation Documentation related issues and PRs - requests, fixes, proposals. label Oct 26, 2017

pamil reviewed Oct 26, 2017

View reviewed changes

stefandoorn force-pushed the docs-plugins branch 2 times, most recently from 2cc4b37 to db122e6 Compare November 7, 2017 20:34

pamil added this to the 1.0 milestone Nov 8, 2017

pamil changed the base branch from master to 1.0 November 8, 2017 09:41

pamil changed the base branch from 1.0 to master November 8, 2017 09:41

pamil reviewed Nov 8, 2017

View reviewed changes

pamil mentioned this pull request Nov 8, 2017

Follow new naming convention Sylius/PluginSkeleton#56

Merged

stefandoorn force-pushed the docs-plugins branch from db122e6 to 8e1ee0d Compare November 8, 2017 17:59

Plugin naming convention RFC implementation (closes Sylius#8848)

d9e315d

stefandoorn force-pushed the docs-plugins branch from 8e1ee0d to d9e315d Compare November 8, 2017 18:01

stefandoorn changed the base branch from master to 1.0 November 8, 2017 18:04

pamil approved these changes Nov 9, 2017

View reviewed changes

pjedrzejewski merged commit 1cfcf9c into Sylius:1.0 Nov 9, 2017

lsmith77 reviewed Nov 9, 2017

View reviewed changes

stefandoorn mentioned this pull request Nov 13, 2017

Plugin naming conventions: DI services & Commands #8959

Closed

pamil mentioned this pull request Feb 2, 2018

Plugin naming conventions #8010

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Plugin naming convention RFC implementation (closes #8848) #8903

Plugin naming convention RFC implementation (closes #8848) #8903

stefandoorn commented Oct 25, 2017

pamil Oct 26, 2017

stefandoorn Oct 26, 2017

pamil Oct 26, 2017

stefandoorn Oct 26, 2017

CoderMaggie Oct 27, 2017

pamil Oct 27, 2017

stefandoorn Oct 27, 2017

pamil Nov 6, 2017

stefandoorn Nov 6, 2017

pamil Nov 6, 2017

pamil Oct 26, 2017

stefandoorn commented Nov 7, 2017

pamil left a comment

pamil Nov 8, 2017

pamil Nov 8, 2017

stefandoorn commented Nov 8, 2017

pamil left a comment

pamil commented Nov 9, 2017

pjedrzejewski commented Nov 9, 2017

lsmith77 Nov 9, 2017

lsmith77 commented Nov 9, 2017

Plugin naming convention RFC implementation (closes #8848) #8903

Plugin naming convention RFC implementation (closes #8848) #8903

Conversation

stefandoorn commented Oct 25, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stefandoorn commented Nov 7, 2017

pamil left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stefandoorn commented Nov 8, 2017

pamil left a comment

Choose a reason for hiding this comment

pamil commented Nov 9, 2017

pjedrzejewski commented Nov 9, 2017

Choose a reason for hiding this comment

lsmith77 commented Nov 9, 2017