From 77ce001836a94ec72c756bd9a407ff30ac03185b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcin=20Kukli=C5=84ski?= Date: Fri, 25 Aug 2023 13:07:50 +0200 Subject: [PATCH 1/6] Update README.md to include new rules --- README.md | 169 ++++++++++++++++++++++++++++++++---------------------- 1 file changed, 102 insertions(+), 67 deletions(-) diff --git a/README.md b/README.md index 3cedbfc..e0915f2 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,7 @@ # Coding bible -*This project follows PSR-4 coding standards and those recommended by Sylius and Symfony projects in this order. It is extended based on the experience of the whole BitBag team for everybody's sake.* +*This project follows PSR-4 coding standards and those recommended by Sylius and Symfony projects in this order. +It is extended based on the experience of the whole BitBag team for everybody's sake.* - [Code Style](#code-style) - [General](#general) @@ -14,11 +15,18 @@ ## Code Style -0. Always follow PSR-4 recommendations; -1. `$elements = [1, 2, 3];` instead of `$elements = array(1, 2, 3);` -2. Don't use annotations. Don't mess up the definition with implementation; -3. Use Yoda-Style comparisons `null === $var->getResult($anotherVar)` instead of `$var->getResult($anotherVar) === null` -4. Don't use PHPDoc. Use it only when it is REALLY valuable and in interfaces that return array of objects or collections, like: +0. Starting a new project, always use the newest stable PHP version. +1. Always follow PSR-4 recommendations. +2. Always prefer newer syntax rules. In example: + - Using `$elements = [1, 2, 3];` instead of `$elements = array(1, 2, 3);` + - Using property / function parameter types, wherever possible + - Using a new constructor syntax known from PHP 8.0 +3. Always use a trailing comma in arrays and method parameters (if PHP version allows to do that). +4. Use Yoda-Style comparisons `null === $var->getResult($anotherVar)` instead of `$var->getResult($anotherVar) === null` +5. Use class constants for static (hardcoded) values. +6. Don't use annotations or attributes for framework configuration. Don't mess up the definition/configuration with implementation. +7. Don't use PHPDoc for things, that can be determined from the code level. Use it only when it is REALLY valuable and in interfaces + that return array of objects or collections, like: ```php interface Foo @@ -30,7 +38,7 @@ interface Foo } ``` -5. Use inline PHPDoc only for fields inside the class, like: +8. Use **inline** PHPDoc only for fields inside the class and method return type (if not specified by the code): ```php final class Foo @@ -38,10 +46,10 @@ final class Foo /** @var int */ private $foo; - /** @var string */ - private $bar; + private string $bar; - public function getFoo(): ?int + /** return int|null */ + public function getFoo() { return $this->foo; } @@ -53,21 +61,21 @@ final class Foo } ``` -6. Keep a blank line above the `@return` method definition in case it has more than `@return` annotation, for instance +9. Keep a blank line above the `@return` method definition in case it has more than `@return` annotation, for instance ```php interface Foo { /** - * @param string $key some valid and important comment + * Some valid and important comment * * @return Collection|ItemInterface[] */ - public function getItemsWithoutKey(string $key): Collection; + public function getItems(string $key): Collection; } ``` -7. Always use strict types declaration in each class header, like: +10. Always use strict types declaration in each class header, like: ```php Preferences`) + and search for `File and Code Templates`. PHP Class Doc Comment, PHP File Header, PHP Interface Doc Comment are those + templates that should at least be customized. -```php -$flavors = [ - 'chocolate', - 'vanilla', -]; -``` +13. Always use [BitBag Coding Standard library](https://github.com/BitBagCommerce/coding-standard) for code cleanup. + Also use PHPStan on level 8 wherever it's possible. It's included in [BitBag Coding Standard library](https://github.com/BitBagCommerce/coding-standard). + Both ECS and PHPStan should be included in the CI process. -10. Good to follow practices from https://mnapoli.fr/approaching-coding-style-rationally/ -11. Once you use PHPStorm (and yes, you do if you work at BitBag), -you can open your IDE preferences (`PHPStorm -> Preferences`) and search for `File and Code Templates`. -PHP Class Doc Comment, PHP File Header, PHP Interface Doc Comment -are those templates that should at least be customized. -12. Always use Easy Coding Standard library for code cleanup. We use one from the official [Sylius Labs repository](https://github.com/SyliusLabs/CodingStandard). If you start the project from scratch, use PHPStan. We use the [official Sylius setup](https://github.com/Sylius/Sylius/blob/master/phpstan.neon). -Both ECS and PHPStan should be included in the CI process. +14. For any point not included in the current section please follow the https://mnapoli.fr/approaching-coding-style-rationally/ tips. ## General -0. No `/.idea` and other local config files in `.gitignore`. Put them into a global gitignore file, read more on https://help.github.com/articles/ignoring-files/#create-a-global-gitignore. -1. We are working on NIX systems and we don't like Windows nor are we solving its existence goal and other problems. -2. Code that is not documented doesn't exist. Writing documentation of a bundle/plugin/project is part of the development process. Remember that in the end, someone else is going to use your code who might not know each part of it. This also applies to writing Github repository descriptions, basic composer package information, etc. +0. No `/.idea` and other local config files in `.gitignore`. Put them into a global gitignore file, + read more on https://help.github.com/articles/ignoring-files/#create-a-global-gitignore. + +1. We are working on NIX systems. We don't like Windows nor are we solving its existence goal and other problems related + to code and application. +2. Code that is not documented doesn't exist. Writing documentation of a bundle/plugin/project is part of the + development process. Remember that in the end, someone else is going to use your code who might not know each part of it. + This also applies to writing GitHub repository descriptions, basic composer package information, etc. + + Specially write/update information of: + - Package installation process + - How to run the application (set of commands/steps needed to do it) + - Information of needed tools, including their versions ## Symfony / Sylius / Frameworks -0. Use YAML (`*.yaml`) for defining routings and configs; -1. Use XML (`*.xml`) for defining services, doctrine, and validation definitions; -2. For services definitions in a single bundle use `form.xml`, `event_listener.xml`, etc. Don't put everything in the `services.xml` file, do it in public projects with only a few services. If you have more than one type of service inside your app, create a separate config file under the `services/` directory. -3. Repositories and Entities in public projects should not (and cannot) be defined as `final`. -4. Entity fields in public projects (vendors) should be `protected` instead of `private`. -5. Decorate resource factories with decoration pattern and do not call resource instance with `new` keyword directly. Instead, inject resource factory into the constructor and call `createNew()` on it. See `Sylius\Component\Product\Factory\ProductFactory`, `sylius.custom_factory.product` service definition and [Symfony Service Decoration](https://symfony.com/doc/current/service_container/service_decoration.html). The `priority` flag we are starting with equals 1 and is increased by one for each other decoration. -6. For customizing forms use [Symfony Form Extension](https://symfony.com/doc/current/form/create_form_type_extension.html). -7. We follow command pattern implemented in [SyliusShopApiPlugin](https://github.com/Sylius/SyliusShopApiPlugin). This means we use the same bus libraries and similar `Command, CommandHandler, ViewRepository, ViewFactory, View` approach. -8. Creating a CLI Command using Symfony Console Component should follow the following rules: +0. Use YAML (`*.yaml`) for defining routings and configs. +1. Use XML (`*.xml`) for defining services, doctrine mappings, and validation definitions. +2. For services definitions in a single bundle use `form.xml`, `event_listener.xml`, etc. Don't put everything. + in the `services.xml` file, do it in public projects with only a few services. If you have more than one type of service. + inside your app, create a separate config file under the `services/` directory. +3. Any information regarding external system (like DSN, service path, credentials) have to been placed in `.env` file as placeholders. +4. Please use `.env.local` file for all sensitive and environment-specific data. +5. Repositories in public projects should not (and cannot) be defined as `final`. +6. Entity fields in public projects (vendors) should be `protected` instead of `private`. +7. Decorate resource factories with decoration pattern and do not call resource instance with `new` keyword directly. + Instead, inject resource factory into the constructor and call `createNew()` on it. + See `Sylius\Component\Product\Factory\ProductFactory`, `sylius.custom_factory.product` service definition + and [Symfony Service Decoration](https://symfony.com/doc/current/service_container/service_decoration.html). The `priority` flag we are starting with equals 1 and is increased by one for each other decoration. +8. For customizing forms use [Symfony Form Extension](https://symfony.com/doc/current/form/create_form_type_extension.html). +9. We follow command pattern. This means we use `Command` / `CommandHandler` / message bus approach. Consider using [Symfony Messenger](https://symfony.com/doc/current/messenger.html) for that. +10. Creating a CLI Command using Symfony Console Component should follow the following rules: - `execute` method should have `int` as a return type. For the **successful** run, the command should return `0`. For any errors during execution, the return can be `1` or any different *error code number*. -9. In Sylius plugins, use traits for customizing models and use them inside your `tests/Application/src` for testing. This way we avoid handling reference conflicts in the final app. -10. We don't use either autowire nor autoconfigure Symfony options as it is a very "magic" way of defining services. We always prefer to manually define services and inject proper arguments into them to have better control of our Container. -11. If some of the service definition is tagged, don't use FQCN (Fully Qualified Class Name) as the service id. -12. Don't use Sylius theme if you have one template in your project. +11. In Sylius plugins, use traits for customizing models and use them inside your `tests/Application/src` for testing. This way we avoid handling reference conflicts in the final app. +12. We don't use either autowire nor autoconfigure Symfony options as it is a very "magic" way of defining services. We always prefer to manually define services and inject proper arguments into them to have better control of our Container. +13. Do not define services as public, if it's not necessary. +14. If some of the service definition is tagged, don't use FQCN (Fully Qualified Class Name) as the service id. +15. Don't use Sylius theme if you have one template in your project. ## Testing -0. Before you implement any new functional feature, write Behat scenario first (Gherkin, `*.feature` file). -1. After writing the scenario, write a proper scenario execution (Contexts, Pages). -2. Use Behat Contexts that are divided into `Hooks` - generic app Background, `Setup` specific resource background, `Ui` - specific interaction. -3. Before starting implementing new functional code, make sure all your core logic is covered with PHPSpec (code without framework dependencies, like Commands, Forms, Configuration, Fixtures, etc.) -4. PHPSpecs are always final classes with `function`s without `public` visibility and `: void` return type: +0. Before starting implementing new functional code, make sure all your core logic is covered with PHPSpec. +1. We use PHPSpecs only for code related to bussiness logic, so please do not write them for controllers, repositories, fixture generators etc. +2. PHPSpecs are always final classes with `function`s without `public` visibility and `: void` return type: ```php final class ProductSpec extends ObjectBehavior @@ -157,13 +177,28 @@ final class ProductSpec extends ObjectBehavior } ``` +3. For integration tests we use PHPUnit with [lchrusciel/api-test-case](https://github.com/lchrusciel/ApiTestCase) library. + The integration tests means testing code with integration to external services, like database, redis, file system. So please write + them for repositories, cache drivers, file uploaders etc. +4. For functional API tests we use PHPUnit with [lchrusciel/api-test-case](https://github.com/lchrusciel/ApiTestCase) library. + The functional API tests means running application API endpoints, comparing their responses with additional database checks. +5. Please write your fixtures using [Nelmio Alice](https://github.com/nelmio/alice) package, which is included in [lchrusciel/api-test-case](https://github.com/lchrusciel/ApiTestCase) library. +6. Before you implement any new functional feature, write Behat scenario first (Gherkin, `*.feature` file). +7. After writing the scenario, write a proper scenario execution (Contexts, Pages). +8. Use Behat Contexts that are divided into `Hooks` - generic app Background, `Setup` specific resource background, `Ui` - specific interaction. ## OOP / Architecture 0. Make your code as simple as it's possible (follow single responsibility principle and KISS principle). -1. Use interfaces for any core logic class implementation, especially Models and Services (so that you follow a single responsibility principle). -2. Use `final` any time it is possible (in order to avoid infinite inheritance chain, in order to customize some parts use Decorator and Dependency Injection patterns). - - The only exception to this rule is only for a framework/library specific requirements. I.e Doctrine Entities cannot be final classes because of reflection issues. +1. Please use the Demeter Law to not to code trains as below: + +```php +$product->getVariants()->first()->getNextThing()->getNextThing()->ohNoImInTheTrain(); +``` + +2. Use interfaces for any core logic class implementation, especially Models and Services (so that you follow a single responsibility principle). +3. Use `final` any time it is possible (in order to avoid infinite inheritance chain, in order to customize some parts use Decorator and Dependency Injection patterns). + - The only exception to this rule is only for a framework/library specific requirements. I.e Doctrine Entities cannot be final classes because of proxy classes existence. 4. Be more careful when you think Singleton is something you need in the project. If it is you should go and rethink the code. 5. Be careful with `static` statement, probably you will never need to use it. 6. Use ADR pattern for controllers. For instance, your controller should not extend any class and contain just an `__invoke` method. It should also be suffixed with `Action` keyword. @@ -197,17 +232,26 @@ final class SayHelloToTheWorldAction ``` -## Workflow +## Workflow and Git + -0. Commit messages should be written (if only it's possible) with the following convention: -`[Project spec state][Bundle] max 64 characters description in english written in Present Simple.` -1. If there is an opened issue on Jira for a specific task, your branch should be named `sit_[ISSUE_NUMBER]`. If not, it should be named with the first letter of your name and your surname. In my case (Mikołaj Król) it would be `mkrol`. +-1. Not confident with Git yet? Visit [the simple guide](http://rogerdudler.github.io/git-guide/)! +0. If you use Git in IDE - make sure it follows all standards. We don't care what GUI/CLI you use as long as you know what happens under the hood. +1. Commit messages should be written (if only it's possible) with the following convention: + `TASK-123 - Max 64 characters description in english written in Present Simple.`. If you don't work with any ticket system, + please skip the `TASK-123 - ` part. How to write a commit message and use Git in a proper way? Read [here](https://github.com/RomuloOliveira/commit-messages-guide). +2. We use Gitflow. So you have to: + - Use correct branch prefixes: `hotfix/`, `feature/`, `refactoring/` etc, + - Follow the Gitflow branching rules described in [the image](doc/gitflow.png), + - If you use any ticket system, use the ticket name in the branch, after prefix. Jira can match branches and tasks together. + + Any kind of change of the flow has to be discussed to the team leader. ## Open Source 0. Open source is made by forks if only more than one person is in charge of maintenance of specific package. -1. We follow http://docs.sylius.org/en/latest/contributing/ contribution standards +1. We follow http://docs.sylius.org/en/latest/contributing/ contribution standards. 2. Any `*.php` file created by the BitBag developer (in Open Source project) needs to have at least the following definition where the author is the user who created this file: ```php @@ -265,15 +309,6 @@ final class SayHelloToTheWorldAction } ``` -## Git - --1. Not confident with Git yet? Visit [the simple guide](http://rogerdudler.github.io/git-guide/)! -0. Use [Gitflow](https://nvie.com/posts/a-successful-git-branching-model/) whenever you can: - -1. How to write a commit message and use Git in a proper way? Read [here](https://github.com/RomuloOliveira/commit-messages-guide) -2. If you use Git in IDE - make sure it follows all standards. We don't care what GUI/CLI you use as long as you know what happens under the hood. - - **Be smart and keep in mind that once you do something stupid, I will find you and I will force you to work with Laravel or Magento.** **There is nothing called a stupid question, but please ask it in a smart way :).** **It's better to talk about a feature with the whole team for 30 minutes than lose 8 hours on implementing some dummy code that will destroy the current codebase order and deep the technical debt.** From e0f55fd2cd13eeac26070b4d7d9eda655aac56ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcin=20Kukli=C5=84ski?= Date: Fri, 25 Aug 2023 13:13:22 +0200 Subject: [PATCH 2/6] Fix a typo for return annotation --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index e0915f2..1eada16 100644 --- a/README.md +++ b/README.md @@ -48,7 +48,7 @@ final class Foo private string $bar; - /** return int|null */ + /** @return int|null */ public function getFoo() { return $this->foo; From 042ecce74dba4aa018a23c7470b8dc064e436887 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcin=20Kukli=C5=84ski?= Date: Fri, 25 Aug 2023 13:14:29 +0200 Subject: [PATCH 3/6] Change NIX typo to *NIX --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 1eada16..91039e0 100644 --- a/README.md +++ b/README.md @@ -124,7 +124,7 @@ public function fooBarIsALongMethodName( 0. No `/.idea` and other local config files in `.gitignore`. Put them into a global gitignore file, read more on https://help.github.com/articles/ignoring-files/#create-a-global-gitignore. -1. We are working on NIX systems. We don't like Windows nor are we solving its existence goal and other problems related +1. We are working on *NIX systems. We don't like Windows nor are we solving its existence goal and other problems related to code and application. 2. Code that is not documented doesn't exist. Writing documentation of a bundle/plugin/project is part of the From d8e1e44e2d34473be782d9b8e0ca67dcfe6e0d8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcin=20Kukli=C5=84ski?= Date: Fri, 25 Aug 2023 13:22:24 +0200 Subject: [PATCH 4/6] Inline PHPDoc, where it needs to be inlined --- README.md | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/README.md b/README.md index 91039e0..13993a8 100644 --- a/README.md +++ b/README.md @@ -31,9 +31,7 @@ It is extended based on the experience of the whole BitBag team for everybody's ```php interface Foo { - /** - * @return Collection|ItemInterface[] - */ + /** @return Collection|ItemInterface[] */ public function getItems(): Collection; } ``` @@ -231,10 +229,8 @@ final class SayHelloToTheWorldAction } ``` - ## Workflow and Git - -1. Not confident with Git yet? Visit [the simple guide](http://rogerdudler.github.io/git-guide/)! 0. If you use Git in IDE - make sure it follows all standards. We don't care what GUI/CLI you use as long as you know what happens under the hood. 1. Commit messages should be written (if only it's possible) with the following convention: From 61d29cbfc6cefbdd91af7107529b12f196fd7433 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcin=20Kukli=C5=84ski?= Date: Mon, 28 Aug 2023 08:22:10 +0200 Subject: [PATCH 5/6] Update the constants rule; Add rule for symfony dependency injection --- README.md | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 13993a8..f0c4649 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,7 @@ It is extended based on the experience of the whole BitBag team for everybody's - Using a new constructor syntax known from PHP 8.0 3. Always use a trailing comma in arrays and method parameters (if PHP version allows to do that). 4. Use Yoda-Style comparisons `null === $var->getResult($anotherVar)` instead of `$var->getResult($anotherVar) === null` -5. Use class constants for static (hardcoded) values. +5. Use class constants / enums for static (hardcoded) values. 6. Don't use annotations or attributes for framework configuration. Don't mess up the definition/configuration with implementation. 7. Don't use PHPDoc for things, that can be determined from the code level. Use it only when it is REALLY valuable and in interfaces that return array of objects or collections, like: @@ -149,15 +149,16 @@ public function fooBarIsALongMethodName( Instead, inject resource factory into the constructor and call `createNew()` on it. See `Sylius\Component\Product\Factory\ProductFactory`, `sylius.custom_factory.product` service definition and [Symfony Service Decoration](https://symfony.com/doc/current/service_container/service_decoration.html). The `priority` flag we are starting with equals 1 and is increased by one for each other decoration. -8. For customizing forms use [Symfony Form Extension](https://symfony.com/doc/current/form/create_form_type_extension.html). -9. We follow command pattern. This means we use `Command` / `CommandHandler` / message bus approach. Consider using [Symfony Messenger](https://symfony.com/doc/current/messenger.html) for that. -10. Creating a CLI Command using Symfony Console Component should follow the following rules: +8. Don't include the entire service container into your service. Instead of that use Symfony Dependency Injection. +9. For customizing forms use [Symfony Form Extension](https://symfony.com/doc/current/form/create_form_type_extension.html). +10. We follow command pattern. This means we use `Command` / `CommandHandler` / message bus approach. Consider using [Symfony Messenger](https://symfony.com/doc/current/messenger.html) for that. +11. Creating a CLI Command using Symfony Console Component should follow the following rules: - `execute` method should have `int` as a return type. For the **successful** run, the command should return `0`. For any errors during execution, the return can be `1` or any different *error code number*. -11. In Sylius plugins, use traits for customizing models and use them inside your `tests/Application/src` for testing. This way we avoid handling reference conflicts in the final app. -12. We don't use either autowire nor autoconfigure Symfony options as it is a very "magic" way of defining services. We always prefer to manually define services and inject proper arguments into them to have better control of our Container. -13. Do not define services as public, if it's not necessary. -14. If some of the service definition is tagged, don't use FQCN (Fully Qualified Class Name) as the service id. -15. Don't use Sylius theme if you have one template in your project. +12. In Sylius plugins, use traits for customizing models and use them inside your `tests/Application/src` for testing. This way we avoid handling reference conflicts in the final app. +13. We don't use either autowire nor autoconfigure Symfony options as it is a very "magic" way of defining services. We always prefer to manually define services and inject proper arguments into them to have better control of our Container. +14. Do not define services as public, if it's not necessary. +15. If some of the service definition is tagged, don't use FQCN (Fully Qualified Class Name) as the service id. +16. Don't use Sylius theme if you have one template in your project. ## Testing From c0338ce9d955a1e839a41e422b91655e7dce2943 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcin=20Kukli=C5=84ski?= Date: Mon, 28 Aug 2023 08:24:28 +0200 Subject: [PATCH 6/6] Edit rule for symfony dependency injection (add 'if you don't have to' sentence) --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index f0c4649..0e64fab 100644 --- a/README.md +++ b/README.md @@ -149,7 +149,7 @@ public function fooBarIsALongMethodName( Instead, inject resource factory into the constructor and call `createNew()` on it. See `Sylius\Component\Product\Factory\ProductFactory`, `sylius.custom_factory.product` service definition and [Symfony Service Decoration](https://symfony.com/doc/current/service_container/service_decoration.html). The `priority` flag we are starting with equals 1 and is increased by one for each other decoration. -8. Don't include the entire service container into your service. Instead of that use Symfony Dependency Injection. +8. Don't include the entire service container into your service, if you don't have to. Instead of that use Symfony Dependency Injection. 9. For customizing forms use [Symfony Form Extension](https://symfony.com/doc/current/form/create_form_type_extension.html). 10. We follow command pattern. This means we use `Command` / `CommandHandler` / message bus approach. Consider using [Symfony Messenger](https://symfony.com/doc/current/messenger.html) for that. 11. Creating a CLI Command using Symfony Console Component should follow the following rules: