Skip to content
This repository has been archived by the owner on Jan 31, 2020. It is now read-only.

Deprecate Attribs and Proxy to Attributes #139

Closed
wants to merge 11 commits into from
Closed

Deprecate Attribs and Proxy to Attributes #139

wants to merge 11 commits into from

Conversation

wandersonwhcr
Copy link
Contributor

@wandersonwhcr wandersonwhcr commented Sep 15, 2017

Remove all references to attribs, translating to attributes. Public methods like setAttribs and getAttribs trigger an E_USER_DEPRECATED and proxy to setAttributes and getAttributes instead.

Fix #87

@@ -302,4 +302,12 @@ public function testSetAttribsDocCommentHasDeprecated()

$this->assertContains('@deprecated', $comment);
}

/**
* @expectedException PHPUnit\Framework\Error\Deprecated
Copy link
Member

Choose a reason for hiding this comment

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

Please look at Travis:

  1. ZendTest\View\Helper\GravatarTest::testGetAttribsIsDeprecated
    ReflectionException: Class PHPUnit\Framework\Error\Deprecated does not exist

https://travis-ci.org/zendframework/zend-view/jobs/275727410#L733

@@ -285,4 +285,12 @@ public function testEmailIsProperlyNormalized()
$this->helper->__invoke('Example@Example.com ')->getEmail()
);
}

/**
* @expectedException PHPUnit\Framework\Error\Deprecated
Copy link
Member

Choose a reason for hiding this comment

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

Please look at Travis:

  1. ZendTest\View\Helper\GravatarTest::testSetAttribsIsDeprecated
    ReflectionException: Class PHPUnit\Framework\Error\Deprecated does not exist

https://travis-ci.org/zendframework/zend-view/jobs/275727410#L730

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. I forgot to comment about it (0:01 AM). I'll fix it right now

Copy link
Member

Choose a reason for hiding this comment

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

I would suggest using method instead of annotation. I know here it doesn't matter because test contain just one call, but I would change it for consistency.

Copy link
Contributor Author

@wandersonwhcr wandersonwhcr Sep 16, 2017

Choose a reason for hiding this comment

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

I don't like annotations, too. But, if I remember, this use is recommended. But I'll change it.

@froschdesign
Copy link
Member

Related to #87

@froschdesign froschdesign added this to the 2.10.0 milestone Sep 15, 2017
@wandersonwhcr
Copy link
Contributor Author

wandersonwhcr commented Sep 15, 2017

I'll create test/phpunit-bc.php (backward compatibility) file that maps PHPUnit\Framework\Error\Deprecated to PHPUnit_Framework_Error_Deprecated, only when needed. After that, I'll change composer.json, adding a files attribute with ["test/phpunit-bc.php"] inside autoload-dev entry.

@froschdesign is that OK? :)

@wandersonwhcr
Copy link
Contributor Author

wandersonwhcr commented Sep 16, 2017

I broke HHVM instances. Let's see what I can do.

@michalbundyra
Copy link
Member

@wandersonwhcr We are dropping hhvm support anyway...

* @param array $attributes
* @return Gravatar
*
* @deprecated Please use Zend\View\Helper\Gravatar::setAttributes
Copy link
Member

Choose a reason for hiding this comment

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

We shouldn't have deprecated tag on that method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops

@michalbundyra
Copy link
Member

@wandersonwhcr

I'll create test/phpunit-bc.php

In other libraries we have test/autoload.php, for example have a look here:
https://github.com/zendframework/zend-modulemanager/blob/master/test/autoload.php

@wandersonwhcr
Copy link
Contributor Author

I rebased my branch and changed source as requested. I hate "do-undo-redo" commits ;)

I'll fix HHVM, just give a couple of hours.

@wandersonwhcr
Copy link
Contributor Author

Done.

weierophinney added a commit that referenced this pull request Jan 17, 2018
Deprecate Attribs and Proxy to Attributes
weierophinney added a commit that referenced this pull request Jan 17, 2018
weierophinney added a commit that referenced this pull request Jan 17, 2018
@weierophinney
Copy link
Member

Thanks, @wandersonwhcr. Merged to develop for release with 2.10.0.

@froschdesign
Copy link
Member

@weierophinney
Is there a reason why you remove all labels after you have closed a PR?

@weierophinney
Copy link
Member

@froschdesign I... didn't? At least, not intentionally. I have no idea why it indicates that at this point. I don't recall doing anything other than closing the window after I was done with this merge; the comment itself was added via the CLI.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants