magento / architecture

A place where Magento architectural discussions happen
275 stars 155 forks source link

PhpStorm settings repository #350

Closed novikor closed 4 years ago

novikor commented 4 years ago

Problem

The difference in PhpStorm code style settings among Magento developers and contributors.

There could be a problem with code formatting standards: the same code which is valid from the static test's point of view can be formatted in different ways.

E.q. space between colon, type hint and method arguments list:

public function execute(int $orderId): void;

vs

public function execute(int $orderId) : void;

Solution

Use PhpStorm Settings repository functionality with required customizations.

As a solution a "Settings Repository" PhpStorm built-in functionality can be used: https://www.jetbrains.com/help/phpstorm/sharing-your-ide-settings.html#settings-repository

It provides a possibility to create Magento-maintained PhpStorm coding standard which all the contributors/core developers will follow. All the code style updates (in the repository itself) will be synchronized with workstations without any manual actions.

There is an existing successful example: https://github.com/novikor/magento2-phpstorm-settings

This repository was created to follow Magento MSI coding standards for less painful PRs code review process.

Requested Reviewers

Lena Orobei (@lenaorobei )

lenaorobei commented 4 years ago

Hi @novikor. I understand the problem you are trying to fix, but I don't think that adding an additional item is a good idea. It is nice to have but is there any way to enforce using it? Another point, what if developer uses different IDE? Many questions to consider and I'm not sure it's worth it. We were also discussing tools like GrumPHP or Captain Hook that work with git hooks and can run some checks before commit/push. This at least does not require any additional configuration.

novikor commented 4 years ago

Well, you are right. There is a similar solution provided by EditorConfig which should fit any IDE, but I did not analyze it.

Thank you for your time.