Skip to content

Conversation

@marcserrat
Copy link
Contributor

No description provided.

@codecov-io
Copy link

codecov-io commented Feb 19, 2020

Codecov Report

Merging #54 into master will decrease coverage by 0.31%.
The diff coverage is 97.42%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #54      +/-   ##
============================================
- Coverage     99.10%   98.79%   -0.32%     
- Complexity      397      481      +84     
============================================
  Files            52       67      +15     
  Lines          1009     1243     +234     
============================================
+ Hits           1000     1228     +228     
- Misses            9       15       +6     
Impacted Files Coverage Δ Complexity Δ
src/ActivationTemplateResponse.php 100.00% <ø> (ø) 1.00 <0.00> (ø)
src/ActivationTileResponse.php 100.00% <ø> (ø) 2.00 <0.00> (ø)
src/Asset.php 100.00% <ø> (ø) 14.00 <0.00> (ø)
src/AutomationEngine.php 100.00% <ø> (ø) 13.00 <0.00> (ø)
src/Config.php 100.00% <ø> (ø) 15.00 <0.00> (ø)
src/Config/VaultConfig.php 100.00% <ø> (ø) 11.00 <0.00> (ø)
src/ConfigException.php 100.00% <ø> (ø) 2.00 <0.00> (ø)
src/ConfigPropertyMissed.php 100.00% <ø> (ø) 1.00 <0.00> (ø)
src/Configuration.php 100.00% <ø> (ø) 1.00 <0.00> (ø)
src/Conversation.php 100.00% <ø> (ø) 3.00 <0.00> (ø)
... and 73 more

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 98a4654...c6aa4b6. Read the comment docs.


use Connect\Model;

class PCBilling extends Model
Copy link
Contributor

Choose a reason for hiding this comment

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

PC prefix is really required here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, 100% due collisions on how Connect models same models in different places

src/Model.php Outdated
case 'object':

$namespaces = ['\Connect\\', '\Connect\Usage\\'];
$namespaces = ['\Connect\\', '\Connect\Usage\\', '\Connect\Product\Capabilities\\', 'Connect\Product\Actions\\', 'Connect\Subscription\\'];
Copy link
Contributor

Choose a reason for hiding this comment

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

line too long? probably worth to put every namespace on own line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK

public function getProduct($productID)
{
$body = $this->sendRequest('GET', '/products/'.$productID);
$body = $this->sendRequest('GET', '/products/' . $productID);
Copy link
Contributor

Choose a reason for hiding this comment

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

why not via constant like in tier-configs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure.....this had been introduced and i'm adopting everywhere when possible....
i added in Directory usage of constants now.

Comment on lines +169 to +175
if ($filters instanceof \Connect\RQL\Query) {
$query = $filters;
} elseif (is_array($filters)) {
$query = new \Connect\RQL\Query($filters);
} else {
$query = new \Connect\RQL\Query();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

probably worth to simplify it as
$query = new \Connect\RQL\Query($filters)
and hide all logic in Query constructor, I seen exactly such block in other places

Comment on lines +219 to +225
if ($filters instanceof \Connect\RQL\Query) {
$query = $filters;
} elseif (is_array($filters)) {
$query = new \Connect\RQL\Query($filters);
} else {
$query = new \Connect\RQL\Query();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

see above about query

Comment on lines +64 to +70
$query = new \Connect\RQL\Query();

if (is_array($filters)) {
$query = new \Connect\RQL\Query($filters);
} elseif ($filters instanceof \Connect\RQL\Query) {
$query = $filters;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

see above about Query

Comment on lines +122 to +129
$query = new \Connect\RQL\Query();

if (is_array($filters)) {
$query = new \Connect\RQL\Query($filters);
} elseif ($filters instanceof \Connect\RQL\Query) {
$query = $filters;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Query again

Comment on lines +208 to +210
if (!$filter) {
$filter = new \Connect\RQL\Query();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Query again

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

97.3% 97.3% Coverage
0.0% 0.0% Duplication

@marcserrat marcserrat merged commit 76f4ee6 into master Mar 19, 2020
@marcserrat marcserrat deleted the v19-dev branch March 19, 2020 05:27
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.

4 participants