Require `const` visibility

[lbajsarowicz]

Rule

Currently const are declared without visibility:
https://github.com/magento/magento2/blob/78bb169ff9721c8d05c35b4c29a4464fd45bccbe/app/code/Magento/Catalog/Model/Product.php#L54-L73

I suggest extending Coding Standard to require visibility - for example:

public const PRIMARY_KEY = 'entity_id';
private const DEFAULT_TIMEOUT = 60;

Reason

const should be explicit if we can use them as a reference or it's declaration is only internal.

Implementation

N/A

[lenaorobei]

I really like this idea but the reason behind this is supported versions. Constants visibility was introduces in PHP 7.1.0 and Magento 2.2.x still supports 7.0.

The idea of coding standard was to have unified set of rules across all release lines. However we had some discussions about version specific rules #17

[lbajsarowicz]

But... We are not going to support 2.2 anymore.
Am I missing some information about EOL of 2.2?

[lenaorobei]

This standard is used for Magento Marketplace extensions verification and i believe patch versions of 2.2.x extensions are still allowed.

[lbajsarowicz]

@lenaorobei Do you think that we can proceed with the rule now?

[lenaorobei]

Hi @lbajsarowicz,

Tagging @vkublytskyi and @roribio who are in charge of Magento Marketplace Extensions Quality Program.

Do we still check 2.2.x extensions?
If no, I think we can move forward with this rule.

[vkublytskyi]

Hi! Good improvement for coding standards! And thank you for notifying us.

At the current moment, Magento Developer Portal still allows submitting extension with declared 2.2 compatibility for Magento Commerce and Magento Commerce Cloud (Magento Open Source allows to specify compatibility only with 2.3 and 2.4 release lines). Therefore we still run tests on obsolete PHP versions to verify declared compatibility with Magento Commerce 2.2.

As Magento 2.4.0 was recently released we are starting the process of fully dropping Magento 2.2 support on Magento Developer Portal. Therefore, I would like to ask you to postpone with merging this rule until we complete that work. I added an internal ticket (MS-7888) to ensure that PR for this issue will be merged in the scope of removal Magento 2.2 from all systems related to submissions of new extensions to Magento Marketplace.

[ihor-sviziev]

@sivaschenko, can it be approved and implemented?

[sivaschenko]

@ihor-sviziev this is a good proposition. I don't see any reason not to implement this rule.

[rmsundar1]

@magento I am working on this