Skip to content

Eliminate unnecessary duplication#102

Closed
photodude wants to merge 15 commits intojoomla:masterfrom
photodude:patch-1
Closed

Eliminate unnecessary duplication#102
photodude wants to merge 15 commits intojoomla:masterfrom
photodude:patch-1

Conversation

@photodude
Copy link
Copy Markdown
Contributor

Since the following files are identical to the source; include the rule set and delete the duplicated file.

  • add Squiz.WhiteSpace.CastSpacing rule to replace CastSpacingSniff.php
  • add Generic.WhiteSpace.DisallowSpaceIndent rule to replace DisallowSpaceIndentSniff.php
  • add Squiz.WhiteSpace.MemberVarSpacing rule to replace MemberVarSpacingSniff.php
  • add Squiz.WhiteSpace.SemicolonSpacing rule to replace SemicolonSpacingSniff.php
  • add PEAR.WhiteSpace.ObjectOperatorIndent rule to replace ObjectOperatorIndentSniff.php
  • add Generic.PHP.LowerCaseConstant rule to replace LowerCaseConstantSniff.php
  • add PSR2.ControlStructures.ElseIfDeclaration rule as an error rather than a warning to replace ElseIfDeclarationSniff.php

@photodude
Copy link
Copy Markdown
Contributor Author

reference issue #104 from @aik099

@aik099
Copy link
Copy Markdown

aik099 commented Apr 9, 2015

I've updated description of #104 several times, so make sure you've addressed all sniffs.

@photodude
Copy link
Copy Markdown
Contributor Author

- [ ] SuperfluousWhitespaceSniff (see issue #104 )
- [ ] ConcatenationSpacingSniff (see issue #104 )

For removals related to PHPCS-2 development branch see PR #101

@aik099
Copy link
Copy Markdown

aik099 commented Apr 9, 2015

Nice catch with these task checkboxes that GitHub have. Would be much easier to track now.

@photodude
Copy link
Copy Markdown
Contributor Author

@mbabker Do you think it would be better to break this PR up into multiple PRs for each file removal and rule inclusion pair? currently it would be about 7 PRs with consideration for 3 others.

@mbabker
Copy link
Copy Markdown
Contributor

mbabker commented Apr 14, 2015

Truthfully, it doesn't matter much to me. As long as I (or someone) can make sense of it all when sitting down to go through it; if it's clearly explained somewhere I'm not too picky about it being one PR.

@photodude
Copy link
Copy Markdown
Contributor Author

@mbabker My thought was it might be easier for you (or someone) to sit down and look at each one individually and approve one here and there; rather than the larger time commitment to review as one big PR.

But taking your advice for this to be "clearly explained somewhere," I have updated the PR description to cover the intent and current inclusions of this PR.

@photodude
Copy link
Copy Markdown
Contributor Author

@mbabker shall we finish this 1.x PR or close in favor of "leaving the 1.x structure in a "minimal maintenance" type of state." ?

@mbabker
Copy link
Copy Markdown
Contributor

mbabker commented Jan 8, 2017

Ya, let's just leave things alone. The whole "if it ain't broke" thing. Better for us to put our code focus on the 2.x ruleset at this point.

@photodude photodude deleted the patch-1 branch March 7, 2017 02:03
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.

3 participants