Skip to content

A few FunctionComment Rules removed#1064

Merged
rthideaway merged 3 commits intodrupal-graphql:8.x-4.xfrom
Dylan203:fix/cs-fixes-iteration-5
Aug 11, 2020
Merged

A few FunctionComment Rules removed#1064
rthideaway merged 3 commits intodrupal-graphql:8.x-4.xfrom
Dylan203:fix/cs-fixes-iteration-5

Conversation

@Dylan203
Copy link
Copy Markdown
Contributor

exclude name="Drupal.Commenting.FunctionComment.ReturnCommentIndentation"
exclude name="Drupal.Commenting.FunctionComment.ParamNameNoMatch"
exclude name="Drupal.Commenting.FunctionComment.ParamMissingDefinition"
exclude name="Drupal.Commenting.FunctionComment.ParamCommentIndentation"
exclude name="Drupal.Commenting.FunctionComment.MissingParamName"
exclude name="Drupal.Commenting.FunctionComment.Missing"

@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 11, 2020

Codecov Report

Merging #1064 into 8.x-4.x will decrease coverage by 0.49%.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff              @@
##             8.x-4.x    #1064      +/-   ##
=============================================
- Coverage      50.38%   49.89%   -0.50%     
  Complexity       683      683              
=============================================
  Files            118      118              
  Lines           1820     1820              
=============================================
- Hits             917      908       -9     
- Misses           903      912       +9     
Impacted Files Coverage Δ Complexity Δ
src/Plugin/DataProducerPluginManager.php 100.00% <ø> (ø) 2.00 <0.00> (ø)
src/Plugin/GraphQL/DataProducer/Utility/Seek.php 100.00% <ø> (ø) 2.00 <0.00> (ø)
...c/Plugin/GraphQL/DataProducer/XML/XMLAttribute.php 100.00% <ø> (ø) 1.00 <0.00> (ø)
src/Plugin/GraphQL/DataProducer/XML/XMLContent.php 100.00% <ø> (ø) 3.00 <0.00> (ø)
src/Plugin/GraphQL/DataProducer/XML/XMLParse.php 100.00% <ø> (ø) 1.00 <0.00> (ø)
src/Plugin/GraphQL/DataProducer/XML/XMLXpath.php 100.00% <ø> (ø) 1.00 <0.00> (ø)
src/GraphQL/Resolver/DefaultValue.php 0.00% <0.00%> (-69.24%) 5.00% <0.00%> (ø%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 557a925...814577b. Read the comment docs.

Copy link
Copy Markdown
Contributor

@rthideaway rthideaway left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's adapt method comments

* @param \Drupal\Core\Cache\Context\CacheContextsManager $contextsManager
* @param \Drupal\Core\Cache\CacheBackendInterface $resultCacheBackend
* @param \Drupal\Core\Extension\ModuleHandlerInterface $moduleHandler
* The module handler.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The module handler. comment is describing $moduleHandler parameter, let's move it under it.


/**
* @cover queryMap::resolve
*/
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cover annotation says which part of the code is covered by the tests in the current method. We shouldn't use it here. Here we are fixing Drupal.Commenting.FunctionComment.Missing rule which should be fixed by commenting the method instead. Let's just use:

  /**
   * Map between persisted query IDs and corresponding GraphQL queries.
   */


/**
* @cover queryMap::resolve
*/
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment here as it's the same map


/**
* @cover queryMap::resolve
*/
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment here as it's the same map


/**
* @covers \Drupal\graphql\Plugin\GraphQL\DataProducer\String\Uppercase::resolve
*/
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

method is testing upper case GraphQL data producer, let's change the comment to:

  /**
   * Tests the upper case data producer.
   */

/**
* @cover EntityUuidBufferTest::resolve
*/
public function testEntityUuidBuffer() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  /**
   * Tests the entity UUID buffer.
   */


/**
* @cover InvalidPayloadTest::resolve
*/
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/**

  • Tests the empty payload.
    */


/**
* @cover InvalidPayloadTest::setUp
*/
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  /**
   * {@inheritdoc}
   */


/**
* @cover UserPermissionsContextTest::resolve
*/
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  /**
   * {@inheritdoc}
   */


/**
* @covers ::defaultValue
*/
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  /**
   * Tests the composite default value resolver.
   */

Copy link
Copy Markdown
Contributor

@rthideaway rthideaway left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, thanks!

@rthideaway
Copy link
Copy Markdown
Contributor

@Dylan203 travis build failed, there is one leftover cs violation, let's fix that and then we're good to go

@rthideaway rthideaway merged commit 21dd433 into drupal-graphql:8.x-4.x Aug 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants