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

support http2 #84

Merged
merged 4 commits into from
Jul 26, 2017
Merged

support http2 #84

merged 4 commits into from
Jul 26, 2017

Conversation

jens1o
Copy link
Contributor

@jens1o jens1o commented Jul 25, 2017

I added the binaries files, that will be installed once you install the dependencies, to the gitignore file.

fixes #81

@jens1o
Copy link
Contributor Author

jens1o commented Jul 25, 2017

@staabm Can you help me fix that error, please?

function testRecognizeHttp2() {
$request = Sapi::createFromServerArray([
'SERVER_PROTOCOL' => 'HTTP/2.0',
'REQUEST_URI' => 'bla'
Copy link
Member

@staabm staabm Jul 25, 2017

Choose a reason for hiding this comment

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

Misses REQUEST_METHOD e.g. GET

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed.

.gitignore Outdated
# Binaries
bin/php-cs-fixer*
bin/phpunit*
bin/sabre-cs-fixer*
Copy link
Member

Choose a reason for hiding this comment

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

Those are already contained, see above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But the .bat equalivants aren't. I think I'll fix it.

@jens1o
Copy link
Contributor Author

jens1o commented Jul 26, 2017

Why are the tests still failing?

@staabm
Copy link
Member

staabm commented Jul 26, 2017

see https://travis-ci.org/fruux/sabre-http/jobs/257649768

seems to be a phpunit issue, unrelated to your PR.

@jens1o
Copy link
Contributor Author

jens1o commented Jul 26, 2017

So you're going to merge it soon?

@@ -6,10 +6,10 @@ composer.lock
tests/cov/

# Composer binaries
bin/phpunit
Copy link
Member

Choose a reason for hiding this comment

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

please drop all your changes from the gitignore

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But then I'm going to commit the .bat binaries.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This prevents that I'm committing these files:
image

Copy link
Member

Choose a reason for hiding this comment

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

ahh I see, you are on windows.

@staabm
Copy link
Member

staabm commented Jul 26, 2017

yep

@staabm staabm merged commit d934681 into sabre-io:master Jul 26, 2017
@staabm
Copy link
Member

staabm commented Jul 26, 2017

thx

@jens1o jens1o deleted the jens1o-support-http2 branch July 26, 2017 09:41
@jens1o
Copy link
Contributor Author

jens1o commented Jul 26, 2017

When will you publish a new version? I need this feature 😆

@staabm
Copy link
Member

staabm commented Jul 26, 2017

as I read the trunk changelog, it will be version 5. as this is the only one which will allow BC breaks I need to wait for @evert and what he likes to break in this release.

alternatively we could cherry pick into a 4.x version. lets see what his opinion is.

@jens1o
Copy link
Contributor Author

jens1o commented Jul 26, 2017

Okay, thanks.

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

Successfully merging this pull request may close these issues.

http 2 doesn't get recognized
2 participants