Skip to content

Conversation

@rdeutz
Copy link
Contributor

@rdeutz rdeutz commented May 20, 2018

Pull Request for Issue #20448

Summary of Changes

Just added the trait as other classes do

Testing Instructions

see #20448

@infograf768
Copy link
Member

This does not break #20448 . I would not know how to test it specifically.

abstract class WebApplication extends AbstractWebApplication implements DispatcherAwareInterface
{
use DispatcherAwareTrait, EventAware, IdentityAware;
use DispatcherAwareTrait, EventAware, IdentityAware,ContainerAwareTrait;
Copy link
Contributor

Choose a reason for hiding this comment

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

Add space after comma.

@laoneo
Copy link
Member

laoneo commented May 20, 2018

But then we should remove the trait from the CMSApplication and pass the container to the web application in the constructor.

@rdeutz
Copy link
Contributor Author

rdeutz commented May 20, 2018

@Quy shouldn't phpcs have catch this?

@Quy
Copy link
Contributor

Quy commented May 20, 2018

I don't know. Let's check with @photodude

@photodude
Copy link
Contributor

@Quy @rdeutz, I'm not sure if the PHPCS 1.5.x version currently used on the CMS would catch that missing space after the comma in the use list. I believe the PHPCS2.x and 3.x versions should, but I haven't run a specific test to verify. The PHPCS2.x and 3.x versions are much more accurate and are more likely to catch the code style errors.

We need to finish cleaning up the existing CMS code style violations so we make the move to the PHPCS2.x and 3.x versions. (I've been busy and haven't had time to work on it recently)

@rdeutz rdeutz closed this Jul 20, 2018
@rdeutz rdeutz deleted the add_missing_trait_20448 branch February 20, 2020 22:33
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.

6 participants