Skip to content

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

Merged
keyvanakbary merged 1 commit into
mybuilder:masterfrom
felixcarmona:codestyleAndProtectedSetUp
Aug 27, 2014
Merged

fix codestyle according to psr and changed the visibility of the setUp into protected#3
keyvanakbary merged 1 commit into
mybuilder:masterfrom
felixcarmona:codestyleAndProtectedSetUp

Conversation

@felixcarmona
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown
Contributor

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

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

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

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)

keyvanakbary added a commit that referenced this pull request Aug 27, 2014
fix codestyle according to psr and changed the visibility of the setUp into protected
@keyvanakbary keyvanakbary merged commit 58f6579 into mybuilder:master Aug 27, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants