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

remove .class parameters and start 2.0 preparation #287

Merged
merged 1 commit into from Nov 6, 2017

Conversation

dbu
Copy link
Member

@dbu dbu commented Oct 31, 2017

No description provided.

@ElectricMaxxx
Copy link
Contributor

Nice circulary dependency, shall i add 1.1|2.0 on testing?

Copy link
Contributor

@ElectricMaxxx ElectricMaxxx left a comment

Choose a reason for hiding this comment

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

can you go on @2.1 for symfony/testing here, please.

.travis.yml Outdated
env: SYMFONY_VERSION=2.8.*
- php: 7.1
env: SYMFONY_VERSION=2.8.*
Copy link
Contributor

Choose a reason for hiding this comment

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

two times 2.8.* ?

Copy link
Member Author

Choose a reason for hiding this comment

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

erm. whatever i did, thanks for spotting, deleting this

<parameter key="doctrine_phpcr.odm.form.path.type.class">Doctrine\Bundle\PHPCRBundle\Form\Type\PathType</parameter>

<parameter key="doctrine_phpcr.odm.metadata.driver_chain.class">Doctrine\Common\Persistence\Mapping\Driver\MappingDriverChain</parameter>
<parameter key="doctrine_phpcr.odm.metadata.annotation.class">Doctrine\ODM\PHPCR\Mapping\Driver\AnnotationDriver</parameter>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can not remove those as Symfony-Doctrine-Bridge still uses those .class parameters. to create the mapping driver stuff

Copy link
Member Author

Choose a reason for hiding this comment

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

i do not see DoctrinePhpcrMappingsPass or the abstract class use any .class parameters. then again, i did not see where the DoctrinePhpcrMappingsPass is even registered at all. did you see where that happens?

Copy link
Contributor

Choose a reason for hiding this comment

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

It was quite hard to find them. They live in AbstractDoctrineExtension of symfony/doctrine-bridge. I greped for anotation or .class the complete parameter id is concatenated in php except .class

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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


 +
 +        <parameter key="doctrine_phpcr.odm.metadata.annotation.class">Doctrine\ODM\PHPCR\Mapping\Driver\AnnotationDriver</parameter>
 +        <parameter key="doctrine_phpcr.odm.metadata.driver_chain.class">Doctrine\Common\Persistence\Mapping\Driver\MappingDriverChain</parameter>
 +        <parameter key="doctrine_phpcr.odm.cache.array.class">Doctrine\Common\Cache\ArrayCache</parameter>

let the tests run again.

Copy link
Member Author

Choose a reason for hiding this comment

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

ah, got it. added the other cache related parameters back as well, as they look used in the extension. this is one of the uglier pieces of symfony code ...

Copy link
Contributor

Choose a reason for hiding this comment

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

I will add a note.

@ElectricMaxxx
Copy link
Contributor

I am behind your branch with mine and recognized some missing class parameters which are used in AbstractDcotrineExtension of doctrine bridge. I got exceptions like:

[Symfony\Component\DependencyInjection\Exception\ParameterNotFoundException]  
  The service "doctrine_phpcr.odm.default_annotation_metadata_driver" has a d   
  ependency on a non-existent parameter "doctrine_phpcr.odm.metadata.annotati   
  on.class".             

I added 3 removed class parameters, you can find them on my diff against yours.

@ElectricMaxxx
Copy link
Contributor

ElectricMaxxx commented Nov 6, 2017

Ahh, cool i got the same: https://travis-ci.org/doctrine/DoctrinePHPCRBundle/jobs/297847476
now it yours :-)
I have no clue where those classes are used.

@dbu
Copy link
Member Author

dbu commented Nov 6, 2017

i think new versions of doctrine jumped to support only php 7.1 and better. i am actually considering the same for phpcr-odm to stay in sync with the rest of the doctrine universe. if we do that, we can drop php 5 here as well. wdyt?

@ElectricMaxxx
Copy link
Contributor

👍 for dropping php 5
using symfony 4.0 + will lead us to use php 7.1 + later

- remove .class parameters
- drop php 5 support
@dbu
Copy link
Member Author

dbu commented Nov 6, 2017

yay, looks green now!

Copy link
Contributor

@ElectricMaxxx ElectricMaxxx left a comment

Choose a reason for hiding this comment

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

yes

@ElectricMaxxx ElectricMaxxx merged commit 678d4b2 into master Nov 6, 2017
@dbu dbu deleted the drop-class-parameters branch November 6, 2017 08:30
ElectricMaxxx added a commit that referenced this pull request Nov 6, 2017
remove .class parameters and start 2.0 preparation

remove duplicate line
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

2 participants