GraphQLBundle icon indicating copy to clipboard operation
GraphQLBundle copied to clipboard

[RFC] Modules naming conventions

Open murtukov opened this issue 4 years ago • 22 comments

Q A
Bug report? no
Feature request? no
BC Break report? no
RFC? yes
Version/Branch 1.0

Before starting to separate the bundle into modules, we need to decide naming conventions.

This discussion addresses following topics:

  1. Naming on GitHub
  2. Naming on Packagist (for composer require command)
  3. Naming of the bundle class, e.g. OverblogGraphQLBundle
  4. Namespaces
  5. Naming of configs

Some rules from Symfony best practices for reusable bundles:

  • Bundle name should not contain more than 2 words + suffix Bundle
  • The bundle class name should be used as the name of the repository (AcmeBlogBundle and not BlogBundle for instance).
  • Symfony core Bundles do not prefix the Bundle class with Symfony and always add a Bundle sub-namespace; for example: Symfony\Bundle\FrameworkBundle\FrameworkBundle.

If module defines it's own services, than it should be a normal Symfony bundle, otherwise it can be a generic php library.

Questions:

  • Should we include the prefix "graphql" to module names? Including it makes names sometimes too long, but not including it deprives the module of information that it deals with GraphQL.

    Examples:

    • overblog/profiler-bundle or overblog/graphql-profiler-bundle
    • overblog/graphql-expression-language-bundle or overblog/graphql-expression-language-bundle
    • OverblogProfilerBundle or OverblogGraphqlProfilerBundle (this breaks the best practices rule "no more than 2 words")
  • Should we rename the core bundle and in what? Suggestion: overblog/graphql-core-bundle

  • What namespaces should we use? Current namespace is Overblog/GraphQLBundle Suggested: Oveblog/GraphQL/CoreBundle, Oveblog/GraphQL/ProfilerBundle etc.

  • What config filenames and key names should we use? Current filename is graphql.yaml and key name is overblog_graphql. Suggestion:

    • Option 1: overblog_graphql.core, overblog_graphql.profiler, overblog_graphql.expression_language
    • Option 2: overblog.graphql.core, overblog.graphql.profiler, overblog.graphql.expression_language
    • Option 3: overblog.graphql_core, overblog.graphql_profiler, overblog.graphql_expression_language

Please write your opinions and suggestions as detailed as possible.

murtukov avatar Jan 25 '21 19:01 murtukov

Questions:

  • Should we include the prefix "graphql" to module names? Including it makes names sometimes too long, but not including it deprives the module of information that it deals with GraphQL.

I think that overblog/profiler-bundle doesn't really make sense. Overblog is a well known organisation (at least in France) and the term overblog doesn't describe at all the topic. If I had to make the call, I would move the entire project to a dedicated organisation with a name related to Symfony & GraphQL (something like SymfoQL or GraphFony for example). But otherwise, I prefer longer but more precise names like overblog/graphql-profiler-bundle.

  • Should we rename the core bundle and in what? Suggestion: overblog/graphql-core-bundle

I think I would keep the actual name for the main bundle as all the other will be plugins to it. So it's easier to understand that overblog/graphql-profiler-bundle is an extension to overblog/graphql-bundle. Otherwise, if I would look into the list of Overblog repositories, I wouldn't be sure what is the main bundle.

  • What namespaces should we use? Current namespace is Overblog/GraphQLBundle Suggested: Oveblog/GraphQL/CoreBundle, Oveblog/GraphQL/ProfilerBundle etc.

Your suggestion is fine by me. Everything should start with Overblog\GraphQL\ and the rest depending on the bundle/library.

  • What config filenames and key names should we use? Current filename is graphql.yaml and key name is overblog_graphql. Suggestion:

    • Option 1: overblog_graphql.core, overblog_graphql.profiler, overblog_graphql.expression_language
    • Option 2: overblog.graphql.core, overblog.graphql.profiler, overblog.graphql.expression_language
    • Option 3: overblog.graphql_core, overblog.graphql_profiler, overblog.graphql_expression_language

I think I prefer when the filename = the key (for simplicity). so I would go for overblog_graphql_xxx with underscores only.

Vincz avatar Jan 25 '21 22:01 Vincz

@Vincz @mcg-web

here you can find the Profiler Module: https://github.com/murtukov/graphql-profiler, it is a draft for now. I added you both as collaborators. Make any suggestions you think make sense. @Vincz one tests fails there, would be nice if you could fix it

murtukov avatar Jan 25 '21 23:01 murtukov

Overblog is a well known organisation (at least in France) and the term overblog doesn't describe at all the topic.

Well the first part is reserved for organisation name and isn't supposed to mean anything :)

murtukov avatar Jan 25 '21 23:01 murtukov

Overblog is a well known organisation (at least in France) and the term overblog doesn't describe at all the topic.

Well the first part is reserved for organisation name and isn't supposed to mean anything :)

Yes, you're right. But sometimes it does and I like it. Like doctrine for example or api-plaform.

Vincz avatar Jan 25 '21 23:01 Vincz

@Vincz @mcg-web

here you can find the Profiler Module: https://github.com/murtukov/graphql-profiler, it is a draft for now. I added you both as collaborators. Make any suggestions you think make sense. @Vincz one tests fails there, would be nice if you could fix it

Hi @murtukov! So I fixed the tests and remove the ones related to missing twig & profiler.

Vincz avatar Jan 26 '21 08:01 Vincz

@Vincz @mcg-web

Ok, here is what I came up with, while working on the Profile module.

Namespaces:

First part - organisation, second part - GraphQL, third part - descriptive name of the module. The reason why I suggest to use CoreBundle for the main bundle is that it avoids dublicate usage of the word GraphQL, e.g. Overblog\GraphQL\GraphQLBundle\GraphQLBundle.

Examples:

  • Overblog\GraphQL\CoreBundle
  • Overblog\GraphQL\ProfilerBundle
  • Overblog\GraphQL\ValidationBundle
  • Overblog\GraphQL\ExpressionLanguageBundle
  • Overblog\GraphQL\AnnotationBundle

Bundle classnames:

Similar to Symfony conventions, the Bundle classnames don't use the organisation name as prefix.

Examples:

  • Overblog\GraphQL\CoreBundle\GraphQLBundle
  • Overblog\GraphQL\ProfilerBundle\GraphQLProfilerBundle
  • Overblog\GraphQL\ValidationBundle\GraphQLValidationBundle
  • Overblog\GraphQL\ExpressionLanguageBundle\GraphQLExpressionLanguageBundle
  • Overblog\GraphQL\AnnotationBundle\GraphQLAnnotationBundle

GitHub repositories:

According to Symfony best practices, the second part of the GitHub name must be identical to the bundle's classname.

Examples:

  • overblog / GraphQLBundle
  • overblog / GraphQLProfilerBundle
  • overblog / GraphQLValidationBundle
  • overblog / GraphQLExpressionLanguageBundle
  • overblog / GraphQLAnnotationBundle

Composer packages:

Packagist names are same as the repository names, but in kebab-case

Examples:

  • composer require overblog/graphql-bundle
  • composer require overblog/graphql-profiler-bundle
  • composer require overblog/graphql-validation-bundle
  • composer require overblog/graphql-expression-language-bundle
  • composer require overblog/graphql-annotation-bundle

If you agree with these rules, we can start to create repositories for the modules.

murtukov avatar Jan 26 '21 15:01 murtukov

I like it. This way it's clear that everything related to GraphQL in the Overblog organisation starts with Overblog/GraphQL. And the fact that the main bundle has the shortest name illustrate properly in my mind that other bundles are "extensions" of this one. Thanks @murtukov!

Vincz avatar Jan 26 '21 15:01 Vincz

How I see that:

  • Package and repository: overblog/gaphql
  • Bundles Paths: src/Bundle/{ProfilerBundle,ValidationBundle,ExpressionLanguageBundle,AnnotationBundle}
  • Naming conventions: Overblog\GraphQL\Bundle\ProfilerBundle
  • Every bundle has a dedicated readonly repository overblog/graphql-profiler-bundle so every bundle should be totally standalone (including tests).
  • We could also include the core bundle or keep a side like today.

Just like https://github.com/symfony/symfony/tree/5.x/src/Symfony/Bundle this will ease the maintainance.

This is not so different from yours @murtukov .

mcg-web avatar Jan 27 '21 13:01 mcg-web

@mcg-web

Package and repository: overblog/gaphql

Please clarify here. What do you mean with "package"? As I understand overblog/graphql is a monolith repository for all bundles like symfony / symfony, so it cannot be installed like composer require overblog/graphql

murtukov avatar Jan 27 '21 16:01 murtukov

package, mean that overblog/graphql can also be used like a meta package, installing a full version of the "graphql-bundle".

mcg-web avatar Jan 27 '21 16:01 mcg-web

@mcg-web full version like installing the bundle with all possible modules? I don't know, I don't see a situation where I would need to install the full version, especially when there are too many modules. I would rather install overblog/graphql-bundle and then install everything i need, e.g.: overblog/graphql-validation-bundle and overblog/graphql-annotations-bundle

murtukov avatar Jan 27 '21 16:01 murtukov

this is just an option like symfony/website-skeleton vs symfony/skeleton. We don't all use a bundle the same way. This option will also help passing from the present bundle to the new split version easier .

mcg-web avatar Jan 27 '21 17:01 mcg-web

@mcg-web yes, but symfony/website-skeleton is not a bundle, that's why it's allowed to omit the bundle suffix. overblog/graphql is a bundle and thus should have the suffix

murtukov avatar Jan 27 '21 17:01 murtukov

No it is not a bundle , it is a meta package including many bundles. Bundles will be enabled using flex recipe.

mcg-web avatar Jan 27 '21 17:01 mcg-web

@mcg-web I think it will be more clear when we create it. Can you start with it? I would like to upload the Profiler module

murtukov avatar Jan 27 '21 18:01 murtukov

Done ! You can create bundles in src/Bundle/. I will work on implement the meta package after .

https://github.com/overblog/graphql

mcg-web avatar Jan 27 '21 18:01 mcg-web

@mcg-web

such paths don't contain the word graphql: src/Bundle/{ProfilerBundle,ValidationBundle,ExpressionLanguageBundle,AnnotationBundle}

murtukov avatar Jan 27 '21 20:01 murtukov

Then let's go to src/GraphQL/Bundle, anyway with composer autoloader and Psr4 this will not affect namespace prefix.

mcg-web avatar Jan 27 '21 20:01 mcg-web

In this case it is even longer, than paths of Symfony bundles:

Symfony\Bundle\FrameworkBundle\ - 3 parts Overblog\GraphQL\Bundle\ProfilerBundle\ - 4 parts

Let's think why did they use these parts and why do we use it.

Symfony:

  • Symfony - organisation, obvious
  • Bundle - to indicate that everything under this path is a bundle, because it could also be a component, bridge or whatever.
  • FrameworkBundle - describes the unique feature. I don't know why they use the Bundle suffix tho, when it's already under Symfony\Bundle

Overblog:

  • Overblog - organisation.
  • GraphQL - to indicate that everything under this path has to do with GraphQL.
  • Bundle - to indicate that everything under this path is a graphql bundle. We will also have non-bundle modules, in this case they should be under Component.
  • ProfilerBundle - describes the unique feature. Again, maybe we shouldn't use the Bundle suffix, why is it needed?

If we omit the Bundle prefix, then we will have something like this:

  • Overblog\GraphQL\Bundle\Profiler\
  • Overblog\GraphQL\Component\Promise\

So it still contains 4 parts, but it's accordingly shorter and provides all necessary information without duplicates. What do you think?

murtukov avatar Jan 27 '21 20:01 murtukov

I prefer too without the Bundle suffix. Useless to repeat the Bundle part IMHO.

Vincz avatar Jan 27 '21 21:01 Vincz

Bundle prefix is to organize (vs Component) and the suffix Bundle it's the framework naming convention.

mcg-web avatar Jan 27 '21 21:01 mcg-web

Yeah, you are right, here is a quote from Symfony Best Practices:

A bundle is also a PHP namespace. The namespace must follow the PSR-4 interoperability standard 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 Bundle.

murtukov avatar Jan 27 '21 22:01 murtukov