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.

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

Move the tests directory to PSR-4 autoloading #332

Merged
merged 2 commits into from
Jan 29, 2020
Merged

Move the tests directory to PSR-4 autoloading #332

merged 2 commits into from
Jan 29, 2020

Conversation

jdufresne
Copy link
Contributor

@jdufresne jdufresne commented Jan 15, 2020

This is first step towards moving the package to a namespace and the composer PSR-4 autoloader. The "classmap" autoloader is no longer used for tests.

As the tests are not part of the public API, this is backwards compatible.

Uses the PhpCas namespace.

Refs #188. Closes #331.

@phy25
Copy link
Member

phy25 commented Jan 15, 2020

I am assuming that this PR includes #331. Once this gets merged, we don't need to address #331 anymore right?

(I will review it closer later since I am quite busy recently, but I would say the time to merge this would be important since this is kind of a breakthrough; thanks in advance!)

@jdufresne
Copy link
Contributor Author

I am assuming that this PR includes #331. Once this gets merged, we don't need to address #331 anymore right?

Yup this is right. It builds on the other PR. If this gets merged/accepted I can close/drop the other one.

(I will review it closer later since I am quite busy recently, but I would say the time to merge this would be important since this is kind of a breakthrough; thanks in advance!)

Thanks! I have rebased to resolve merge conflicts.

Copy link
Member

@phy25 phy25 left a comment

Choose a reason for hiding this comment

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

Thank you for the refactoring! LGTM.

I think I found some issues with the legacy autoload code during the review, but this is definitely out of the scope of this review. I will address it with a PR, which will be helpful when we continue PSR-4 refactoring towards source folder.

CAS.php Show resolved Hide resolved
@phy25
Copy link
Member

phy25 commented Jan 20, 2020

BTW this needs to be rebased again to be merged, sorry about this.

@jdufresne
Copy link
Contributor Author

BTW this needs to be rebased again to be merged, sorry about this.

Done.

For details on composer autoload, see:

https://getcomposer.org/doc/04-schema.md#autoload

This helps move from a custom maintained autoload function to a standard
one built by composer. Composer's autoload function is widely used by
the larger PHP community and is well tested. To maintain backwards
compatibility, Autoload.php is still included, but emits a warning.

This helps moves the project towards adopting the community standard
PSR-4 autoloading:

https://www.php-fig.org/psr/psr-4/
Copy link
Member

@phy25 phy25 left a comment

Choose a reason for hiding this comment

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

Almost done.

test/CAS/Tests/AuthenticationTest.php Show resolved Hide resolved
test/CAS/Tests/AuthenticationTest.php Outdated Show resolved Hide resolved
This is first step towards moving the package to a namespace and the
composer PSR-4 autoloader. The "classmap" autoloader is no longer used
for tests.

As the tests are not part of the public API, this is backwards
compatible.

Uses the PhpCas namespace.

Refs #188
@phy25 phy25 merged commit 93c07e0 into apereo:master Jan 29, 2020
@jdufresne jdufresne deleted the tests-psr4 branch January 29, 2020 04:12
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.

2 participants