Fix deprecated route options #614
Conversation
soullivaneuh
force-pushed
the
deprecated-routes
branch
from
fc524ad
to
8559a5a
Compare
|
Not sure failing tests are due to my PR. Can you check that? Thanks. |
|
@soullivaneuh could you please rebase your PR? Tests should be back to green now, at least on the |
soullivaneuh
force-pushed
the
deprecated-routes
branch
from
8559a5a
to
c9b35b1
Compare
|
Done. 👍 |
|
Now you can fix your code because tests are failling :p |
|
Damn it! I'll take a look. ;-) |
|
@willdurand I can't figure why test failed. Not sure if is the test that are outdated or a real error. Can you take a look and give me your advice please? |
|
ping @willdurand |
|
don't exactly know what happens to be honest.. |
|
It looks like it's only the order of the method that have changed (for some reason ?). We can either find why the order changed, or change the expected results. |
soullivaneuh
force-pushed
the
deprecated-routes
branch
2 times, most recently
from
6e58b74
to
1c82b7c
Compare
|
@magnetik I suspected that too and you I right. I fixed one test by reverting some test data. See here: https://github.com/nelmio/NelmioApiDocBundle/pull/614/files#diff-bb9610de031bd0610a5f213da720a4c7L91 But I have no idea of why test fixtures order changed. I review my modifications twice and didn't find any mistake... @willdurand Have you already saw this? I'll work on other tests right now. |
soullivaneuh
force-pushed
the
deprecated-routes
branch
from
1c82b7c
to
8edfe51
Compare
|
@willdurand Test fixed by reverting data on expected results. Good to merge if OK with that. |
|
@soullivaneuh, I had the same issue with the test's order in #616. I still haven't any clue about the reason. |
|
Ok now all fails except on SF 2.6. @willdurand I think you have magic trolling random test suites... :-D |
soullivaneuh
force-pushed
the
deprecated-routes
branch
from
8edfe51
to
b564f23
Compare
|
@willdurand Branch rebased since #640. Should be OK now. 👍 |
|
Could you please rebase your PR one more time? |
|
Are you kidding me? :-) Sure I will. 👍 |
soullivaneuh
force-pushed
the
deprecated-routes
branch
from
b564f23
to
6b81f87
Compare
|
@willdurand It's done. But I don't know if test are working. Didn't resolve anything since last time... |
|
@willdurand Again the same fails... Routing order changed with Symfony 2.,7, not for other. Can't figure out why. |
|
would be nice to get this wrapped up ... |
|
@lsmith77 I agree. If you have any clue about failing test, I'm hearing! :-) |
soullivaneuh
force-pushed
the
deprecated-routes
branch
from
6b81f87
to
f127a1a
Compare
|
Would be good if we can get this merged... |
soullivaneuh
force-pushed
the
deprecated-routes
branch
from
f127a1a
to
d84f69e
Compare
|
I tend to agree with @lsmith77, @soullivaneuh, and @teohhanhui. It'd be nice to have this merged or have an update on when it might happen. I recently updated to Symfony 2.7.x and I'm getting tons of these messages from this bundle. |
|
@NeuralClone We got some strange issue on Travis test for the moment. If you have any clue to solve it I would be glad to apply a patch quickly. |
|
@soullivaneuh I've taken a closer look at this and I'm having trouble figuring out why the tests are failing as well. I don't think it's anything you've introduced, though. I'm not seeing any obvious mistakes in your changes, and the same tests aren't passing in the master branch for me. Exact same reasons for the failures as well. If they're passing for other people, I'm questioning what it is they're doing differently. Unfortunately, I realize this doesn't really come any closer to helping to resolve this. I wish I had something more concrete to report. I'll keep playing with it when I have some free time. :) |
|
IMO the failing tests shouldn't block this PR, since it's the fault of the maintainers themselves... In other words, it's not this PR's responsibility to fix it. |
|
At the very least I think a new PR should be created to fix the tests since the problems with them seem to go beyond the scope of what this PR is trying to achieve. And this PR doesn't appear to have introduced the problems in the first place. |
This is the problem. Those erros didn't appear until this PR. But I can't just figure out why. |
The builds have been failing for quite some time now... https://travis-ci.org/nelmio/NelmioApiDocBundle/builds |
|
@teohhanhui Maybe. But see job details, errors are not the same. |
|
ping @soullivaneuh |
|
@mvhirsch The PR is ready. buggy tests are not completely related. |
|
merged, thanks |
@willdurand Are you sure? Seems to be only closed here. |
|
yep. it is merged by hand, not pushed yet though. |
|
Okay thanks. |
|
Indeed. Thank you for merging this. :) |
This PR fix deprecated routing config throwing related errors:
We have to bump
symfony/framework-bundleto~2.2version. Not a big deal because 2.1 is not maintained anymore.