Skip to content

use namespaced PHPUnit and keep B.C.#55

Merged
Seldaek merged 7 commits into
php-fig:masterfrom
DQNEO:namespaced-phpunit
Oct 25, 2019
Merged

use namespaced PHPUnit and keep B.C.#55
Seldaek merged 7 commits into
php-fig:masterfrom
DQNEO:namespaced-phpunit

Conversation

@DQNEO

@DQNEO DQNEO commented Apr 2, 2018

Copy link
Copy Markdown
Contributor

This is an alternate solution to #52

Comment thread Psr/Log/Test/LoggerInterfaceTest.php Outdated

namespace Psr\Log\Test;

if (!class_exists('\\PHPUnit_Framework_TestCase', true)) {

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.

This does not make sense. You are creating an alias only when the old name does not exist (i.e. in PHPUnit 6+) while you use only the new name.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sorry, my bad...
I fixed it. Could you review it again?

@stof

stof commented Apr 3, 2018

Copy link
Copy Markdown
Contributor

currently, the repo does not have a .gitattribute to exclude the tests from the archive, so every project installing psr/log (most of them today) will have this file with the class_alias in it, which would mess the IDE autocompletion in projects.

So I suggest either keeping this alias out (PHPUnit namespaced classes are available in 4.8.35+, 5.4+ and 6+) or excluding tests from the archive.

@Seldaek what do you think ?

@DQNEO DQNEO closed this Apr 4, 2018
@DQNEO DQNEO deleted the namespaced-phpunit branch April 4, 2018 16:31
@DQNEO DQNEO restored the namespaced-phpunit branch April 4, 2018 18:02
@DQNEO DQNEO reopened this Apr 4, 2018
@Seldaek

Seldaek commented Nov 20, 2018

Copy link
Copy Markdown
Collaborator

IMO at this point we could merge it without the alias.. This only affects implementors of the PSR which isn't a whole lot of people.

Comment thread Psr/Log/Test/LoggerInterfaceTest.php Outdated

namespace Psr\Log\Test;

if (!class_exists('\\PHPUnit\\Framework\\TestCase', true)) {

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.

Class names in PHP shouldn't have the leading slash when written in a string. The implementation of class_exist effectively does an ltrim to remove that slash before continuing. The reason why you might see leading slashes is when the ::class notation is used, because then we are resolving names relative to the current namespace.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks! fixed. e50f69c

@michalbundyra

Copy link
Copy Markdown

@Seldaek Any chance we'll get it sorted finally?
Per your latest comment I guess we want #52 back.

@Seldaek

Seldaek commented Aug 24, 2019

Copy link
Copy Markdown
Collaborator

Looks good to me now, @stof is the new alias ok or does your comment above still apply regarding IDE confusion?

@Seldaek Seldaek merged commit bf73deb into php-fig:master Oct 25, 2019
@DQNEO DQNEO deleted the namespaced-phpunit branch October 25, 2019 09:18
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.

5 participants