Skip to content
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

Jump to bottom

Update FunctionComment sniff with phpcs 2 fixes #159

Merged
merged 5 commits into from
Feb 28, 2017
Merged

Update FunctionComment sniff with phpcs 2 fixes #159

merged 5 commits into from
Feb 28, 2017

Conversation

photodude
Copy link
Contributor

@photodude photodude commented Feb 19, 2017 •
edited
Loading

@photodude
Copy link
Contributor Author

photodude commented Feb 19, 2017 •
edited
Loading

holding off on the changes related to correctly handle multi-line param comments squizlabs/PHP_CodeSniffer@7e92909 as they might not apply to our sniff due to our changes

@photodude photodude mentioned this pull request Feb 19, 2017
Open
6 tasks
@wilsonge
Copy link
Contributor

So is this ok to merge then?

@photodude
Copy link
Contributor Author

I think this can be merged. Do you want to review the referenced phpcs change? I think it doesn't apply but I'm not 100% sure.

@photodude photodude changed the title Update with phpcs 2 fixes Update FunctionComment sniff with phpcs 2 fixes Feb 20, 2017
@photodude
Copy link
Contributor Author

I've partially added the changes from squizlabs/PHP_CodeSniffer@7e92909 but due to our Joomla specific changes in the sniff I can't see how the rest of the changes would apply.

mbabker
mbabker reviewed Feb 28, 2017
View reviewed changes
Joomla/Sniffs/Commenting/FunctionCommentSniff.php Outdated
@@ -231,7 +235,8 @@ protected function processParams(PHP_CodeSniffer_File $phpcsFile, $stackPtr, $co
{
if ($tokens[$i]['code'] === T_DOC_COMMENT_STRING)
{
$comment .= ' ' . $tokens[$i]['content'];
$comment .= ' ' . $tokens[$i]['content'];
$commentEnd = $i;
Copy link
Contributor

Choose a reason for hiding this comment

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

CS is wrong 😉

Copy link
Contributor Author

@photodude photodude Feb 28, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Ugh, Stupid github web interface and their enforced tabs=8 spacing making it hard to see things when you set it back to tab=4.

Should be fixed...
(although I'm not sure why that didn't flag in the rules check... maybe something isn't working or we need another sniff for that.)

@mbabker mbabker merged commit f22e460 into joomla:phpcs-2 Feb 28, 2017
@photodude photodude deleted the patch-2 branch February 28, 2017 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mbabker mbabker mbabker left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@photodude @wilsonge @mbabker