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

ZF3 compatibility #168

Merged
merged 11 commits into from Sep 30, 2016
Merged

ZF3 compatibility #168

merged 11 commits into from Sep 30, 2016

Conversation

michalbundyra
Copy link
Member

Added ZF3 compatibility but also kept ZF2 compatibility. PHP5.5 support dropped.

There is no any BC breaks ! ! ! 😄

It depends on doctrine/DoctrineModule#567

The only problem was with zend-mvc-console (over ZF3) so please see suggest section in composer.json and travis configuration.

Thoughts?

@@ -1,3 +1,3 @@
vendor
composer.lock
Copy link
Member

Choose a reason for hiding this comment

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

Our other libraries don't commit composer.lock either, so I'd suggest leaving it out for the module as well.

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 think it is better to add the lockfile now, to have wider spectrum of tests, so we can make sure the library is compatible with all dependencies. Please have a look on comments here:
zendframework/zend-inputfilter#107 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

I'll let @gianarb decide on this one. Personally I'd be in favour of not committing the lock file and running tests with latest dependencies (as is the default in composer install without a lock file) and once with the --prefer-lowest flag set to install the lowest dependencies. This has multiple advantages: it makes sure the lower package constraints are still correct, and it would alleviate the problem Matthew indicated in the linked comment about developers not knowing if their PR broke due to their code changes or upstream package changes.

"zendframework/zend-servicemanager": "2.*",
"zendframework/zend-stdlib": "2.*"
"php": "~5.6 || ~7.0",
"doctrine/doctrine-module": "dev-master",
Copy link
Member

Choose a reason for hiding this comment

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

Just a reminder that it would be much, much better if we had a stable version of doctrine-module we can require at this point.

Copy link
Member Author

Choose a reason for hiding this comment

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

of course, once doctrine-module will be tagged I'll update it here, or it could be done before release :) thanks

Copy link
Contributor

Choose a reason for hiding this comment

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

Huh? It's already tagged, isn't it?

Copy link
Member

Choose a reason for hiding this comment

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

Not the one including doctrine/DoctrineModule#567, which includes compatibility with ZF3.

Copy link
Member Author

Choose a reason for hiding this comment

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

No :( not yet, @gianarb merged DoctrineModule and DoctrineORMModule without tagging.
Please see his comment here: doctrine/DoctrineORMModule#491 (comment)

@michalbundyra
Copy link
Member Author

@gianarb what do you think about this PR? Can you review it and merge it please?

@aight8
Copy link

aight8 commented Sep 14, 2016

Any news?

@respinoza
Copy link

@webimpress Is there anything we can do to help you out with this PR ? As far as I can tell the DoctrineModule supports ZF3 (thank you for the work) but waiting to be tagged.

@michalbundyra
Copy link
Member Author

@respinoza This is ready, I think it could be merged to dev-master as others doctrine modules are. And probably it can be tagged after DoctrineModule release. I haven't experienced any issues in this library so far.

@gianarb gianarb merged commit a8afacd into doctrine:master Sep 30, 2016
@gianarb
Copy link
Contributor

gianarb commented Sep 30, 2016

Thanks @webimpress for your AMAZING work to help the doctrine community to enjoy ZF3.

I have in plan to tag all our modules soon because we have a good amount of tests now 👍

@respinoza
Copy link

Thank you for the merge.

@michalbundyra michalbundyra deleted the feature/zf3compat branch December 13, 2018 22:58
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

7 participants