Skip to content
This repository was archived by the owner on Aug 1, 2024. It is now read-only.

Symfony 4 support#276

Merged
gilles-g merged 19 commits intolexik:masterfrom
bodorszilard:symfony-4-support
Mar 12, 2018
Merged

Symfony 4 support#276
gilles-g merged 19 commits intolexik:masterfrom
bodorszilard:symfony-4-support

Conversation

@bodorszilard
Copy link
Copy Markdown
Contributor

#SymfonyConHackday2017

#SymfonyConHackday2017
#SymfonyConHackday2017
#SymfonyConHackday2017
#SymfonyConHackday2017
#SymfonyConHackday2017
#SymfonyConHackday2017
@bodorszilard
Copy link
Copy Markdown
Contributor Author

the unit tests need to be fixed (it's not related to this pr)

.travis.yml Outdated
matrix:
exclude:
- php: 5.6
env: DEPENDENCIES='beta' SYMFONY_VERSION='4.0.*' STABLE=beta
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You can remove beta, Symfony 4 is stable now

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@garak DONE

@garak
Copy link
Copy Markdown
Contributor

garak commented Dec 4, 2017

I'm afraid this won't work until you add explicit visibility of services

@gilles-g
Copy link
Copy Markdown
Member

@garak You can see tests are green on SF3.2 and PHP 7.1, but not with 3.3. And not with PHP 7.0
https://travis-ci.org/lexik/LexikFormFilterBundle/builds/272870684

I'm trying to search why we have these errors, but right now, I don't know.. 🤔

But the tests were already broken on previous commits..

@garak
Copy link
Copy Markdown
Contributor

garak commented Dec 13, 2017

I see all tests failing. But, again, you need to fix that service visibility. Otherwise, even if you fix tests, Symfony 4 won't work

@gilles-g
Copy link
Copy Markdown
Member

Yes the service id lexik_form_filter.query_builder_updater needs to be public and @bodorszilard can update his PR with this fix.

@garak
Copy link
Copy Markdown
Contributor

garak commented Dec 13, 2017

I'm afraid you need to make the listener public, too

use PHPUnit\Framework\TestCase;

class PrepareListenerTest extends \PHPUnit_Framework_TestCase
class PrepareListenerTest extends TestCase
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If you want to switch to newer phpunit version, you must be sure to avoid deprecated functions. E.g., $this->getMock() has been removed

@garak
Copy link
Copy Markdown
Contributor

garak commented Dec 27, 2017

Any news on this PR?

@FriTOol
Copy link
Copy Markdown

FriTOol commented Jan 11, 2018

Any news?

$myPlatform = $this->getMockBuilder('Doctrine\DBAL\Platforms\MySqlPlatform')->getMock();

$connection = $this->getMock(
$connection = $this->getMockClass(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe getMockBuilder()? And then getMock() on the result.

'',
false
);
$connection = $this->getMockBuilder('Doctrine\DBAL\Connection')->getMock();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You also need disableOriginalConstructor() here

@flaksp
Copy link
Copy Markdown

flaksp commented Feb 5, 2018

Any news?

@Alex17000
Copy link
Copy Markdown

Any news? :)

@garak
Copy link
Copy Markdown
Contributor

garak commented Feb 13, 2018

Still waiting for tests to pass. Until @bodorszilard doesn't make proper changes, I'm afraid that this cannot be merged

@gilles-g
Copy link
Copy Markdown
Member

I have tried to fix some tests, but some tests failed and I don't know why and I'm not a specialist of DateRange with different Timezone 😜

  1. Lexik\Bundle\FormFilterBundle\Tests\Filter\Doctrine\DBALQueryBuilderUpdaterTest::testDateRangeWithTimezone
    Failed asserting that two strings are equal.
    --- Expected
    +++ Actual
    @@ @@
    -'SELECT i FROM item i WHERE (i.startAt <= '2015-10-20 23:59:59') AND (i.startAt >= '2015-10-20 00:00:00')'
    +'SELECT i FROM item i WHERE (i.startAt <= '2015-10-20 20:59:59') AND (i.startAt >= '2015-10-19 21:00:00')'

@cbeyer
Copy link
Copy Markdown

cbeyer commented Feb 16, 2018

I'm also waiting :)

@qtsd qtsd mentioned this pull request Feb 22, 2018
@odinsey
Copy link
Copy Markdown

odinsey commented Mar 8, 2018

Any news ? Can't use this bundle in sf3.4 without forking it, just because a service isn't public.
Please Wake Up Lexik Agency

@brunonic
Copy link
Copy Markdown

brunonic commented Mar 9, 2018

Someone ?

@gilles-g
Copy link
Copy Markdown
Member

@garak What did you think? We can merge PR maybe

@garak
Copy link
Copy Markdown
Contributor

garak commented Mar 12, 2018

@Spike31 it' OK for me

@gilles-g gilles-g merged commit e248745 into lexik:master Mar 12, 2018
@gilles-g
Copy link
Copy Markdown
Member

gilles-g commented Mar 12, 2018

Thanks for all your help @bodorszilard and the others of course :)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants