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
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions .gitignore
@@ -1,3 +1,3 @@
vendor
composer.lock
.idea
vendor/
.idea/
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.

phpunit.xml
64 changes: 50 additions & 14 deletions .travis.yml
@@ -1,27 +1,63 @@
sudo: false

language: php

php:
- 5.5
- 5.6
cache:
directories:
- $HOME/.composer/cache
- vendor

env:
- MONGO_DRIVER=mongo
global:
- COMPOSER_ARGS="--no-interaction"
- ADAPTER_DEPS="alcaeus/mongo-php-adapter"
- LATEST_DEPS="zendframework/zend-mvc-console"
- MONGO_DRIVER=mongo

matrix:
fast_finish: true
include:
- php: 7.0
env: ADAPTER_VERSION="^1.0.0" MONGO_DRIVER=mongodb
- php: 5.6
env:
- DEPS=lowest
- php: 5.6
env:
- DEPS=locked
- TEST_COVERAGE=true
- php: 5.6
env:
- DEPS=latest
- php: 7
env:
- DEPS=lowest
- MONGO_DRIVER=mongodb
- php: 7
env:
- DEPS=locked
- CS_CHECK=true
- MONGO_DRIVER=mongodb
- php: 7
env:
- DEPS=latest
- MONGO_DRIVER=mongodb

services: mongodb

before_script:
before_install:
- travis_retry composer self-update

install:
- yes '' | pecl -q install -f $MONGO_DRIVER
- if [ "x${ADAPTER_VERSION}" != "x" ]; then composer require "alcaeus/mongo-php-adapter=${ADAPTER_VERSION}" --ignore-platform-reqs; fi
- composer install --prefer-source
- if [[ $MONGO_DRIVER == 'mongodb' ]]; then travis_retry composer require --dev $COMPOSER_ARGS $ADAPTER_DEPS --ignore-platform-reqs ; fi
- if [[ $DEPS == 'latest' ]]; then travis_retry composer require --dev --no-update $COMPOSER_ARGS $LATEST_DEPS ; fi
- if [[ $DEPS == 'latest' ]]; then travis_retry composer update $COMPOSER_ARGS ; fi
- if [[ $DEPS == 'lowest' ]]; then travis_retry composer update --prefer-lowest --prefer-stable $COMPOSER_ARGS ; fi
- travis_retry composer install $COMPOSER_ARGS
- composer show

script:
- cd tests
- cp test.application.config.php ../config/application.config.php
- ../vendor/bin/doctrine-module odm:schema:create
- ../vendor/bin/doctrine-module odm:schema:drop
- ../vendor/bin/phpunit
- if [[ $DEPS == 'latest' ]]; then cp ./tests/TestConfigurationV3.php ./config/application.config.php ; else cp ./tests/TestConfigurationV2.php ./config/application.config.php ; fi
- ./vendor/bin/doctrine-module odm:schema:create
- ./vendor/bin/doctrine-module odm:schema:drop
- if [[ $TEST_COVERAGE == 'true' ]]; then ./vendor/bin/phpunit --coverage-clover clover.xml ; else ./vendor/bin/phpunit ; fi
- if [[ $CHECK_CS == 'true' ]]; then ./vendor/bin/phpcs --standard=PSR2 ./src/ ./tests/ ./config/ ; fi
23 changes: 15 additions & 8 deletions composer.json
Expand Up @@ -38,16 +38,23 @@
}
],
"require": {
"php": "~5.5 || ~7.0",
"doctrine/doctrine-module": "~1.0",
"doctrine/mongodb-odm": "~1.0",
"zendframework/zend-mvc": "2.*",
"zendframework/zend-servicemanager": "2.*",
"zendframework/zend-stdlib": "2.*"
"php": "~5.6 || ~7.0",
Copy link
Member

Choose a reason for hiding this comment

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

Please don't align dependencies, this just creates unnecessary diffs due to re-alignment when dependencies change.

Copy link
Member Author

Choose a reason for hiding this comment

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

hm .... https://github.com/doctrine/DoctrineModule/blob/master/composer.json#L33-L56
I'd like just to be consistent with your other libraries ;) What do you think then?

Copy link
Member

Choose a reason for hiding this comment

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

If it's up to me, no. I'll let @gianarb decide since he's the official maintainer of this module.

Choose a reason for hiding this comment

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

I'd prefer ^5.6 || ^7.0 notation

"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)

"doctrine/mongodb-odm": "^1.1",
"zendframework/zend-mvc": "^2.7.10 || ^3.0.2",
"zendframework/zend-servicemanager": "^2.7.6 || ^3.1.1",
"zendframework/zend-stdlib": "^2.7.7 || ^3.0.1"
},
"require-dev": {
"zendframework/zendframework": "2.*",
"phpunit/phpunit": "~4.8 || ~5.0"
"phpunit/phpunit": "^4.8",
"squizlabs/php_codesniffer": "^2.6.2",
"zendframework/zend-console": "^2.6",
"zendframework/zend-i18n": "^2.7.3",
"zendframework/zend-log": "^2.9",
"zendframework/zend-modulemanager": "^2.7.2",
"zendframework/zend-serializer": "^2.8",
"zendframework/zend-session": "^2.7.3",
"zendframework/zend-view": "^2.8.1"
},
"autoload": {
"psr-0": {
Expand Down