Skip to content

fix codestyle according to psr and changed the visibility of the setUp into protected #3

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.

Sign up for GitHub

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

Merged
keyvanakbary merged 1 commit into from
Aug 27, 2014
Merged

fix codestyle according to psr and changed the visibility of the setUp into protected #3

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions tests/TestListenerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ class TestListenerTest extends \PHPUnit_Framework_TestCase
private $listener;
private $dummyTest;

public function setUp()
protected function setUp()
{
$this->listener = new TestListener;
$this->dummyTest = new DummyTest;
$this->listener = new TestListener();
Copy link
Copy Markdown
Contributor

@keyvanakbary keyvanakbary Aug 26, 2014

Choose a reason for hiding this comment

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

Correct me if I'm wrong but this is not part of the PSR, right? We don't have a clear convention about this, only personal preferences. Personally, I really like it as it was, without parenthesis. Could you recover it? :)

Copy link
Copy Markdown
Contributor

@sp-felix-carmona sp-felix-carmona Aug 26, 2014

Choose a reason for hiding this comment

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

You are right, the PSR don't say about this, but the cs-fixer by Fabien Potencier fixes it.
https://github.com/fabpot/PHP-CS-Fixer
new_with_braces [all] All instances created with new keyword must be followed by braces.
https://github.com/fabpot/PHP-CS-Fixer/blob/master/Symfony/CS/Fixer/NewWithBracesFixer.php
Here is the discussion: https://github.com/fabpot/PHP-CS-Fixer/issues/384 (I have a git hook that fixes my code before commits, this is the reason that I changed it), if you want, I can remove these () for you.

Copy link
Copy Markdown
Contributor

@keyvanakbary keyvanakbary Aug 27, 2014

Choose a reason for hiding this comment

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

It would be nice to know the reason behind that decision (consistency?). I've been thinking about this and, as this library attempts to be adopted by everyone, maybe is a good time to adopt a popular convention like that even if I'm not agree with some parts of it. Thanks! :)

Copy link
Copy Markdown
Contributor

@sp-felix-carmona sp-felix-carmona Aug 27, 2014

Choose a reason for hiding this comment

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

I asked it here: https://github.com/fabpot/PHP-CS-Fixer/issues/384#issuecomment-53536535
Under the "Choose from the list of available fixers:" in the readme (https://github.com/fabpot/PHP-CS-Fixer) you can see other non-psr standards (symfony standards, and others (they are marked with [all] tag)

$this->dummyTest = new DummyTest();
}

/**
Expand Down Expand Up @@ -47,4 +47,4 @@ class PHPUnit_Fake extends \PHPUnit_Framework_TestCase
class DummyTest extends \PHPUnit_Fake
{
public $property = 1;
}
}