Skip to content

Misleading message for constant comment #164

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
krisdante opened this issue Dec 14, 2019 · 1 comment
Closed

Misleading message for constant comment #164

krisdante opened this issue Dec 14, 2019 · 1 comment
Labels
bug Something isn't working

Comments

@krisdante
Copy link

Preconditions

The following code comes from Image component:

    /**
     * Default font size
     */
    const DEFAULT_FONT_SIZE = 15;

Steps to reproduce

Test with phpcs
vendor/bin/phpcs --standard=Magento2 lib/internal/Magento/Framework/Image

Expected result

It shall pass

Actual result

FILE: 
lib/internal/Magento/Framework/Image/Adapter/AbstractAdapter.php
----------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------------------
 65 | WARNING | Constants must have short description if they add information beyond what the constant name supplies.
----------------------------------------------------------------------------------------------------------------------

This is misleading information. The code has a description. Also the constant is self-explanatory.

The test checks if the comment is not equal to the variable itself. That leads to situation that the comment needs to be rephrased to something like "Default size for the font" just to pass this test,

@krisdante krisdante added the bug Something isn't working label Dec 14, 2019
@lenaorobei
Copy link
Contributor

@krisdante please see my comment here #163 (comment)

This is the expected behavior. There is no need to duplicate the constant name in the DocBlock.

Documentation: https://devdocs.magento.com/guides/v2.3/coding-standards/docblock-standard-general.html#constants

That leads to situation that the comment needs to be rephrased to something like "Default size for the font" just to pass this test

If the short description adds no additional information beyond what the constant name already supplies, the short description must be omitted.

magento-devops-reposync-svc pushed a commit that referenced this issue Mar 24, 2022
…nto-coding-standard-379

[Imported] AC-2394 Update eslint npm dependency
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants