Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

[WIP] Simplify transport #1

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
2 participants

marijn commented Apr 15, 2012

Composition of the HTTP message is now handled by the underlying Symfony\Component\HttpFoundation\Response object.

As a result cookies should now be properly set. Unfortunately I'm having trouble getting the test suite to work, the whole suite fails due to a RuntimeException with no message. As a result, I'm not able to confirm if this breaks any existing tests.

Is there any specific setup required for the test suite?

marijn commented Apr 15, 2012

Also note that I've been pretty naive with this PR. It was the result of a quick read through the source code. It's likely that I've missed the motivations for the previous implementation.

Owner

pminnieur commented Apr 15, 2012

I haven't tested it against the latest version of Symfony, yet, but will during the week. I've already seen the Response::__toString method which should do fine.

marijn commented Apr 15, 2012

Let me know if any additional work is needed to make this mergeable. I'll dive into the problems with the test suite tomorrow at the office.

Owner

pminnieur commented Apr 15, 2012

It would help if you could tell me where exactly the RuntimeException is thrown.

Owner

pminnieur commented May 1, 2012

Hi marijn, would you mind applying the following patch file to a clean checkout and test it? It works for me, just wanted to make sure it works for you, too. Please do a rm -rf vendor && composer.phar install --dev after applying the patch!

http://pastebin.com/TwsybKBA

marijn commented May 1, 2012

Hi @pminnieur, I'm having trouble applying the patch:

$ patch -p0 -i patch.diff --dry-run
can't find file to patch at input line 5
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/.travis.yml b/.travis.yml
|index 78851a0..577e898 100644
|--- a/.travis.yml
|+++ b/.travis.yml
--------------------------
File to patch: .travis.yml
patching file .travis.yml
can't find file to patch at input line 17
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/composer.json b/composer.json
|index 92ac99c..f6601e0 100644
|--- a/composer.json
|+++ b/composer.json
--------------------------
File to patch: composer.json
patching file composer.json
can't find file to patch at input line 60
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/composer.lock b/composer.lock
|index 874e762..6266d8a 100644
|--- a/composer.lock
|+++ b/composer.lock
--------------------------
File to patch: composer.lock
patching file composer.lock
can't find file to patch at input line 192
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/src/Leach/Container/SymfonyContainer.php b/src/Leach/Container/SymfonyContainer.php
|index ef8cca1..5d2e936 100644
|--- a/src/Leach/Container/SymfonyContainer.php
|+++ b/src/Leach/Container/SymfonyContainer.php
--------------------------
File to patch: src/Leach/Container/SymfonyContainer.php
patching file src/Leach/Container/SymfonyContainer.php
can't find file to patch at input line 223
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/src/Leach/Transport.php b/src/Leach/Transport.php
|index 358174f..d4f71f9 100644
|--- a/src/Leach/Transport.php
|+++ b/src/Leach/Transport.php
--------------------------
File to patch: src/Leach/Transport.php
patching file src/Leach/Transport.php
Hunk #1 FAILED at 215.
1 out of 1 hunk FAILED -- saving rejects to file src/Leach/Transport.php.rej
can't find file to patch at input line 268
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/tests/Leach/Tests/TransportTest.php b/tests/Leach/Tests/TransportTest.php
|index 07c49d2..0cbd993 100644
|--- a/tests/Leach/Tests/TransportTest.php
|+++ b/tests/Leach/Tests/TransportTest.php
--------------------------
File to patch: tests/Leach/Tests/TransportTest.php
patching file tests/Leach/Tests/TransportTest.php
patch unexpectedly ends in middle of line
Hunk #1 succeeded at 196 with fuzz 2.
Owner

pminnieur commented May 1, 2012

okay, try this branch instead: https://github.com/pminnieur/Leach/tree/response

marijn commented May 15, 2012

I'm closing this PR in favor of your https://github.com/pminnieur/Leach/tree/response branch.

I think my issues with the test suite are related to the lack of a socket connection. I'll have to investigate that some other time, too busy at the moment with some other stuff.

Where can I find you online for Leach related questions?

@marijn marijn closed this May 15, 2012

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