Coding standards

Created on 28 September 2018, about 6 years ago
Updated 10 August 2024, 4 months ago

Problem/Motivation

In last testing results we've got 155 coding standards messages
Here is full list of messages:
(EDIT: I removed all these messages, as it is not readable or usable the way they were formatted, is out-of-date, and is generally difficult to read and confusing to this issue. The current up-to-date list of coding standards problems may always be found by looking at the output of the automated tests at https://www.drupal.org/node/36041/qa )

Patches should NOT try to fix all issues at one time - instead they should focus on just one type of coding standards problem and fix that one type of problem for Voting API. Note that there are some issues that should probably NOT be fixed, like the line lengths in API.txt and CHANGELOG.txt for instance. Let's focus on getting the important things done first, like missing @param and documentation comments.

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

📌 Task
Status

Closed: outdated

Version

3.0

Component

Code

Created by

🇸🇮Slovenia vdenis Maribor

Live updates comments and jobs are added and updated live.
  • Coding standards

    It involves compliance with, or the content of coding standards. Requires broad community agreement.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.5 + Environment: PHP 7.3 & MySQL 5.7
    last update over 1 year ago
    PHPLint Failed
  • 🇧🇷Brazil Diego_Mow

    Uploading file with latest PHPCS review and generated patch #29 .

    I read notes from #28 several times and those are some considerations while creating this patch:

    1. Drush Commands vs Dependecy Injection: I added Ignore Flags on all so we don't need to go trough this discussion again.
    2. Drupal t(): Also did the same: using Ignore Flags to remove it.
    3. Voting API file: In the end of it there is a abstract example of Storage changing. I tried to fill all "Todo" flags there but also Ignoring that file made sense to me.
    4. VoteTypeAccessControlHandler.php: DID NOTHING AT ALL. My honest opinion is that this should be deleted, so I prefer to investigate it in a new issue instead of this one.
  • Status changed to Needs work over 1 year ago
  • 🇮🇹Italy apaderno Brescia, 🇮🇹

    TR said:

    Yes, the CS warning will remain, and that's OK.

    He did not suggest to add @codingStandardsIgnoreLine to all the lines that must not be changed. He said to ignore the warnings/errors for those lines that should not be changed, or that should be changed in a different issue.

  • Assigned to RohitRawat676
  • Status changed to Active over 1 year ago
  • Issue was unassigned.
  • Status changed to Needs review over 1 year ago
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.5 + Environment: PHP 7.3 & MySQL 5.7
    last update over 1 year ago
    Patch Failed to Apply
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 5.7
    last update over 1 year ago
    Patch Failed to Apply
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 5.7
    last update over 1 year ago
    Patch Failed to Apply
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.1.x + Environment: PHP 7.4 & MySQL 5.7
    last update over 1 year ago
    Patch Failed to Apply
  • Status changed to Needs work over 1 year ago
  • 🇺🇸United States tr Cascadia
  • 🇮🇹Italy apaderno Brescia, 🇮🇹
    diff --git a/entity.module b/entity.module
    index b1a72ef..562e322 100644
    --- a/entity.module
    +++ b/entity.module

    The last patch is not for this project, since there is any entity.module file in this project.

    +/**
    + * @file
    + */
    +
     use Drupal\Core\Entity\ContentEntityInterface;

    Leaving out the patch is not for this project, TR already explained that is the wrong type of changes to do. That documentation comment requires a short description, after the line containing @file; adding just the line containing @file to avoid a PHP_CodeSniffer warning/error is not the type of changes he will accept.

  • First commit to issue fork.
  • First commit to issue fork.
  • Status changed to Needs review 9 months ago
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.5 + Environment: PHP 7.3 & MySQL 5.7
    last update 9 months ago
    9 pass, 2 fail
  • 🇮🇳India chaitanyadessai Goa

    Please review patch.

  • Status changed to Needs review 9 months ago
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.5 + Environment: PHP 7.3 & MySQL 5.7
    last update 9 months ago
    9 pass, 2 fail
  • 🇮🇳India chaitanyadessai Goa

    Please ignore #38.

  • Status changed to Needs review 9 months ago
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.5 + Environment: PHP 7.3 & MySQL 5.7
    last update 9 months ago
    10 pass
  • 🇮🇳India chaitanyadessai Goa

    Please review patch.

  • Status changed to Needs work 9 months ago
  • 🇺🇸United States tr Cascadia

    No, and I really don't feel like repeating myself.

  • 🇮🇹Italy apaderno Brescia, 🇮🇹
     /**
    - * Voting API's vote storage can be overridden by setting the
    - * 'votingapi_vote_storage' state variable to an alternative class.
    + * Voting API's.
      */
    -\Drupal::state()->set('votingapi_vote_storage', 'Mongodb_VoteStorage');
    +// Vote storage can be overridden by .
    +// setting the 'votingapi_vote_storage'state variable to an alternative class.
    +\Drupal::state()->set('votingapi_vote_storage', 'VotingApi');

    Replacing a full sentence with Voting API's is not a correct change, as it is not correct to split a sentence in a phrase and a sentence.
    Furthermore, why is the class name changed?

    +   * @param int $vote
        *   instance of VotingApi_Vote.
        */
       public function addVote(&$vote) {

    If $vote is an object, it cannot be an integer.

  • Status changed to Closed: outdated 4 months ago
  • 🇺🇸United States tr Cascadia

    Please open new issue(s) for any new problems found that show up in GitLab CI test results.

Production build 0.71.5 2024