Skip to content
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

chore: add comment to track actions and common command object #3776

Merged
merged 70 commits into from
Apr 27, 2023

Conversation

@gabrielfs7 gabrielfs7 changed the base branch from master to develop March 31, 2023 09:01
@codecov-commenter
Copy link

codecov-commenter commented Mar 31, 2023

Codecov Report

Merging #3776 (57051c8) into develop (a406392) will increase coverage by 0.63%.
The diff coverage is 44.46%.

@@              Coverage Diff              @@
##             develop    #3776      +/-   ##
=============================================
+ Coverage      27.48%   28.12%   +0.63%     
- Complexity     10794    10881      +87     
=============================================
  Files            903      915      +12     
  Lines          32202    32655     +453     
=============================================
+ Hits            8852     9183     +331     
- Misses         23350    23472     +122     
Impacted Files Coverage Δ
actions/class.ClientConfig.php 0.00% <0.00%> (ø)
actions/class.RdfController.php 0.00% <0.00%> (ø)
helpers/ServiceProvider/HelperServiceProvider.php 0.00% <0.00%> (ø)
helpers/class.Mode.php 0.00% <0.00%> (ø)
helpers/dateFormatter/DateFormatterFactory.php 0.00% <0.00%> (ø)
...asses/clientConfig/ClientConfigServiceProvider.php 0.00% <0.00%> (ø)
models/classes/clientConfig/GetConfigQuery.php 0.00% <0.00%> (ø)
...ureVisibility/FeatureVisibilityServiceProvider.php 0.00% <0.00%> (ø)
models/classes/menu/MenuService.php 0.00% <0.00%> (ø)
models/classes/menu/MenuServiceProvider.php 0.00% <0.00%> (ø)
... and 15 more

... and 2 files with indirect coverage changes

@gabrielfs7 gabrielfs7 requested a review from shpran March 31, 2023 14:42
Comment on lines 71 to 74
/** @var LoggerInterface|MockObject */
private LoggerInterface $logger;

protected function setUp(): void
Copy link
Contributor

Choose a reason for hiding this comment

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

You are declaring the property for the subject under test dynamically on line 87, but I'd be better to declare it explicitly here.

Suggested change
/** @var LoggerInterface|MockObject */
private LoggerInterface $logger;
protected function setUp(): void
/** @var LoggerInterface|MockObject */
private LoggerInterface $logger;
private ClientConfigStorage $sut;
protected function setUp(): void

Comment on lines 116 to 120
$query
->method('getShownStructure')
->willReturn('shownStructure');
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

  • Remember to extend this test or call markTestIncomplete()
Suggested change
$query
->method('getShownStructure')
->willReturn('shownStructure');
}
}
$query
->method('getShownStructure')
->willReturn('shownStructure');
$this->markTestIncomplete();
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it is in progress yet by @shpran . We should test at least the happy path here.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, makes sense 👌 sorry for the noise

composer.json Outdated Show resolved Hide resolved
Comment on lines 115 to 119

public function setConfigByPath(array $path): void
{
$this->config = array_merge_recursive($this->config, $path);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@shpran

Suggested change
public function setConfigByPath(array $path): void
{
$this->config = array_merge_recursive($this->config, $path);
}
public function setConfigByPath(array $path): void
{
$this->config = array_merge_recursive($this->config, $path);
}

@@ -7,6 +7,7 @@ tao-core
- [Middlewares Doc](models/classes/Middleware/README.md)
- [Feature Flag](models/classes/featureFlag/README.md)
- [CSRF Tokens](models/classes/security/xsrf/README.md)
- [Client Config](models/classes/clientConfig/README.md)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

🚀 Thanks @shpran

@shpran shpran force-pushed the feat/RFE-530/integration-branch branch from a8efd58 to 4811cb2 Compare April 18, 2023 09:00
@github-actions
Copy link

Version

Target Version 52.1.0
Last version 52.0.0

There are 0 BREAKING CHANGE, 11 features, 9 fixes

@gabrielfs7 gabrielfs7 merged commit 7a52a92 into develop Apr 27, 2023
@gabrielfs7 gabrielfs7 deleted the feat/RFE-530/integration-branch branch April 27, 2023 13:36
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.

5 participants