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

PSR-12 #2907

Merged
merged 4 commits into from Jan 2, 2020
Merged

PSR-12 #2907

merged 4 commits into from Jan 2, 2020

Conversation

@t0mmy742
Copy link
Contributor

t0mmy742 commented Dec 31, 2019

PSR-12 expanded and replaced PSR-2. Can Slim switch to PSR-12 ?
PSR-12: Extended Coding Style

I also add fully-qualified function calls (use function xxxx). I didn't add 5 functions (headers_sent, header, connection_status, is_readable and is_writable) since we override them during tests (used on ResponseEmitter and Routing/RouteCollector classes).

@coveralls

This comment has been minimized.

Copy link

coveralls commented Dec 31, 2019

Coverage Status

Coverage remained the same at 100.0% when pulling 158afb5 on t0mmy742:psr12 into 539c334 on slimphp:4.x.

@t0mmy742

This comment has been minimized.

Copy link
Contributor Author

t0mmy742 commented Jan 1, 2020

I didn't add 5 functions (headers_sent, header, connection_status, is_readableand is_writable) since we override them during tests (used on ResponseEmitter and Routing/RouteCollector classes).

To complete this PR, I created a "sub-branch". You can have a diff just here. I'm using uopz extension to change the return value of our 5 functions during tests.
Let me know what you prefer : using uopz extension (it does NOT slow down our tests) or never call our functions with fully-qualified names. Then, I can merge or not this "sub-branch".

@t0mmy742 t0mmy742 force-pushed the t0mmy742:psr12 branch from cc5a3d4 to 9547af3 Jan 2, 2020
@l0gicgate

This comment has been minimized.

Copy link
Contributor

l0gicgate commented Jan 2, 2020

This also effectively resolves #2881 right? I'm in favor of this. Will need to do on Slim-Http, Slim-Psr7 and Slim-Skeleton

I don't see any added value from using the UOPZ extension, I suppose it makes the code more uniform. What do you think @adriansuter? This PR works both ways.

@l0gicgate l0gicgate added this to the 4.4.0 milestone Jan 2, 2020
@l0gicgate l0gicgate added the Slim 4 label Jan 2, 2020
@adriansuter

This comment has been minimized.

Copy link
Contributor

adriansuter commented Jan 2, 2020

Yes, let's move to PSR-12.

Concerning the override, I once started a project that could be used to override global (root) namespace functions. But it is not completed yet.

https://github.com/adriansuter/php-autoload-override

The idea is, that we can define classes for which we can define functions that should be overridden. The tool then intercepts the loading of these classes (using stream wrappers) and overwrites the function calls. It actually works.

@t0mmy742 t0mmy742 force-pushed the t0mmy742:psr12 branch from 3bff092 to 158afb5 Jan 2, 2020
@t0mmy742

This comment has been minimized.

Copy link
Contributor Author

t0mmy742 commented Jan 2, 2020

Yes it closes #2881

@adriansuter your library is amazing, and really easy to use. It is what we need here !

Do you want to keep all that code in the bootstrap.php file ? Or move code in Assets folder ?

@l0gicgate

This comment has been minimized.

Copy link
Contributor

l0gicgate commented Jan 2, 2020

I think what we can do for now is merge this and we can decide/finish the rest in a different PR. The scope of this PR is already pretty big.

@l0gicgate l0gicgate merged commit cf68c2d into slimphp:4.x Jan 2, 2020
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 100.0%
Details
@l0gicgate l0gicgate mentioned this pull request Jan 5, 2020
@t0mmy742 t0mmy742 deleted the t0mmy742:psr12 branch Jan 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.