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

Sync to Slim\Http in 4.x #24

Merged
merged 11 commits into from May 14, 2017
Merged

Conversation

akrabat
Copy link
Member

@akrabat akrabat commented May 8, 2017

Pick up all the changes to Slim\Http in Slim 4 by copying the code from Slim 4.x into this repository.

Notes:

  • I've kept the createFromEnvironment to createFromGlobals rename
  • I've deleted the FactoryDefault factory class.
  • I've added PHPCS.
  • I've added PHP 5.6 testing to Travis.
  • I've removed completely base path - we need to handle this in Slim, not Slim-Http

Pending decision on if we’re going to go PHP 7+ only…
Rename createFromEnvironment() to createFromGlobals() and make it take
an array, rather than an Environment object. This means the Environment
is only required for testing.
@coveralls
Copy link

Coverage Status

Coverage increased (+7.6%) to 98.197% when pulling e609c8a on akrabat:replace-with-slim4x-version into 7407c9e on slimphp:master.

@akrabat akrabat added this to the 0.2 milestone May 8, 2017
@@ -12,7 +12,7 @@
* Headers Interface
*
* @package Slim
* @since 3.0.0
* @since 1.0.0
Copy link
Member

Choose a reason for hiding this comment

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

Should this not be 3.0.0 as it was before? we only got HeadersInterface in Slim 3?

Copy link
Member Author

Choose a reason for hiding this comment

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

My view was that this will be since 1.0 of Slim-Http.

Arguably the package should be Slim-Http though?

@geggleto
Copy link
Member

In theory this looks pretty good.

"psr/http-message": "^1.0"
},
"require-dev": {
"squizlabs/php_codesniffer": "^2.5",
"phpunit/phpunit": "^5.7|^6.0"
},
"conflict": {
"slim/slim": "^3.0"
Copy link
Member

Choose a reason for hiding this comment

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

Should this not be slim/slim ^4.0 as we will have namespacing conflicts?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope. Slim 4 won't have any of the Slim\Http namespace in it.

Copy link
Member

Choose a reason for hiding this comment

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

but say I include slim/http in a Slim 3 project ... ?

Copy link
Member

Choose a reason for hiding this comment

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

Oh my bad... it says conflict xD

@coveralls
Copy link

Coverage Status

Coverage increased (+7.7%) to 98.207% when pulling 9c485cd on akrabat:replace-with-slim4x-version into 7407c9e on slimphp:master.

composer.json Outdated
"phpunit/phpunit": "^5.7|^6.0"
},
"conflict": {
"slim/slim": "^3.0"
},
"autoload": {
"psr-4": {
"Slim\\Http\\": "src",
"Slim\\Http\\": "src"
Copy link
Member

Choose a reason for hiding this comment

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

I think from composer docs it says the path should have a / after it, so it should be "src/"

@akrabat akrabat merged commit 556b628 into slimphp:master May 14, 2017
akrabat added a commit that referenced this pull request May 14, 2017
@coveralls
Copy link

Coverage Status

Coverage increased (+7.7%) to 98.207% when pulling 556b628 on akrabat:replace-with-slim4x-version into 7407c9e on slimphp:master.

This was referenced May 14, 2017
@akrabat akrabat deleted the replace-with-slim4x-version branch May 14, 2017 09:24
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.

None yet

4 participants