Skip to content
New issue

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

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

Already on GitHub? # to your account

EZP-28722: Add unit tests to Compiler Passes #305

Conversation

mikadamczyk
Copy link
Contributor

Question Answer
Tickets https://jira.ez.no/browse/EZP-28722
Bug fix? no
New feature? yes
BC breaks? no
Tests pass? yes
Doc needed? no
License GPL-2.0

Add unit tests to Compiler Passes

Checklist:

  • Coding standards ($ composer fix-cs)
  • Ready for Code Review


class ComponentPass implements CompilerPassInterface
{
const TAG_NAME = 'ezplatform.admin_ui.component';
public const TAG_NAME = 'ezplatform.admin_ui.component';
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be unified across all new PRs? it is not in PSR afair?

Copy link
Contributor

Choose a reason for hiding this comment

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

I wouldn't go with that change in this PR. I see PHPStorm is proposing this change but this will introduce too many inconsistencies in the codebase.

Copy link
Contributor

@webhdx webhdx left a comment

Choose a reason for hiding this comment

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

Nitpicks but would be nice to change.

@@ -16,9 +16,12 @@
*/
class TabPass implements CompilerPassInterface
{
const TAG_TAB = 'ezplatform.tab';
public const TAG_TAB = 'ezplatform.tab';
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.

use Symfony\Component\DependencyInjection\Reference;

/**
* Supplies config Providers to the Aggregator.
*/
class UiConfigProviderPass implements CompilerPassInterface
{
public function process(ContainerBuilder $container)
public const TAG_CONFIG_PROVIDER = 'ezplatform.admin_ui.config_provider';
Copy link
Contributor

Choose a reason for hiding this comment

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

remove public

@mikadamczyk mikadamczyk force-pushed the ezp-28722-add-unit-tests-to-compiler-passes- branch from 4a3a4a0 to eab56e7 Compare January 22, 2018 11:15
@lserwatka lserwatka merged commit bd679ba into ezsystems:1.0 Jan 22, 2018
# for free to join this conversation on GitHub. Already have an account? # to comment
Development

Successfully merging this pull request may close these issues.

4 participants