Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

Updated Travis CI configuration (slack, hhvm, php 7.2, locked, coveralls) + docs rebase #78

Merged
merged 27 commits into from
Apr 25, 2018
Merged

Updated Travis CI configuration (slack, hhvm, php 7.2, locked, coveralls) + docs rebase #78

merged 27 commits into from
Apr 25, 2018

Conversation

michalbundyra
Copy link
Member

@michalbundyra michalbundyra commented Jul 12, 2017

  • removed Slack integration
  • removed build on HHVM
  • added build on PHP 7.2
  • added build on locked version (composer.lock is no longer ignored)
  • removed documentation build configuration (as it is now handled by zendframework/zfbot)
  • rebased docs - renamed documentation directory: doc -> docs

.travis.yml Outdated
- travis_retry composer self-update

install:
- travis_retry composer install $COMPOSER_ARGS
- travis_retry composer install $COMPOSER_ARGS --ignore-platform-reqs
Copy link
Contributor

Choose a reason for hiding this comment

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

just a question, why we need --ignore-platform-reqs in that case?
it was working without that before.

Copy link
Member Author

Choose a reason for hiding this comment

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

Before we didn't have LOCKED dependencies. It is possible that locked dependencies contains some packages not supported on the current PHP version so we need first force install them and then we re-update them in the following lines. The same we have in the other libraries.

Copy link
Contributor

Choose a reason for hiding this comment

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

so that mean on PHP 5.6 locked we use phpunit 5.7.X and on php 7.X locked we use phpunit 6.X.X, from my site i was everytime thinking that the locked tests are to test same dependencies on different PHP version.

personal i create the composer.lock file with composer.json platform php 5.6, to create a lock which should work on all PHP versions.

Copy link
Member

Choose a reason for hiding this comment

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

@kokspflanze What we do when we have PHP 5.6 in the mix is to add an additional line that will downgrade specific dependencies:

  - if [[ $TRAVIS_PHP_VERSION =~ ^5.6 ]]; then travis_retry composer update $COMPOSER_ARGS --with-dependencies $LEGACY_DEPS ; fi

where $LEGACY_DEPS is generally just phpunit/phpunit, as that's the specific dependency we generally have issues with across different PHP versions. This generally works the same as using the lockfile, as we typically only specify require-dev deps in $LEGACY_DEPS.

Copy link
Contributor

Choose a reason for hiding this comment

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

@weierophinney sure i understand how it works, but i dont think its usefull.

if we have a zf-module, which is needed in zend-i18n and which require PHP 7 in version 2 and someone create the lock file he have to add it in $LEGACY_DEPS because in version 1 the module works with PHP 5.6 too, or he have to add other special parts in the travis.yml.
We also dont check the require PHP modules, which maybe not installed, but which needed for some tests. Sure its possible to check in the tests if the module is installed, but if the module not exists, i think its not good to skip these tests.

i think its easier to generate a lock file on the lowest PHP version, which should work on all PHP versions.

correct me if im wrong, or if i missed somthing

Copy link
Member Author

Choose a reason for hiding this comment

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

@weierophinney I can't see any references to phpunit.xml.travis, so maybe this is a residue and we should remove it now?

Copy link
Member Author

Choose a reason for hiding this comment

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

@kokspflanze

i think its easier to generate a lock file on the lowest PHP version, which should work on all PHP versions.

Not sure, we should simplify dev process as much as we can. Probably not everyone has still PHP 5.6, many people already migrated to PHP 7+ and I wouldn't complicate it for them. IMHO we shouldn't force all contributors to generate lock file always on PHP 5.6. Please also bare in mind we are dropping PHP 5 support and all new major releases are going to support only PHP 7.1+ so part with legacy dependencies is going to be removed soon. We have done this way in many packages already, it doesn't cause any issues, I remember issues at the beginning when we started adding support for PHPUnit 6 and lock file was generated on PHP 7. And again, as @weierophinney already said:

This generally works the same as using the lockfile

Copy link
Contributor

@kokspflanze kokspflanze Aug 23, 2017

Choose a reason for hiding this comment

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

@webimpress you can force the php version of the composer.lock generation with

    "config": {
        "platform": {
            "php": "5.6.30"
        }
    },

nobody have to install that old version.

all in all for me i think with --ignore-platform-reqs we can create problems later when different modules require higher PHP versions. also we will never lose the legacy support, if phpunit drop php 7.1 in 2019 or smth we will get the problem again, its just phpunit 7 and phpunit8 =)

Copy link
Member Author

Choose a reason for hiding this comment

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

@kokspflanze

you can force the php version of the composer.lock generation with

I didn't know about it. Nice! Maybe then this is better solution? @weierophinney ?

Copy link
Member

Choose a reason for hiding this comment

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

@webimpress — No - use PHP 7 or 7.1 for the composer.lock generation; otherwise, we're using a lower set of dependencies in many cases due to the fact that some libraries and/or their versions won't work with PHP 5 (e.g., PHPUnit 6). Additionally, we have 2, and shortly to be 3, versions of PHP 7 we test against, but only 1 version of PHP 5 we test against. As such, use the versions supported by the majority of PHP versions we test, not the other way around.

@kokspflanze — we only use --ignore-platform-reqs for the initial install from the lockfile; all other operations take it into account. The line I shared earlier for installing from $LEGACY_DEPS is tested and works well, and ensures we have a consistent installation across that particular PHP version, and that the dependencies work for that PHP version. Further, if we add any additional dependencies that depend on PHP 7+, we'll know about them due to Travis erroring on that particular job, and we can then evaluate whether or not we need to change any of our constraints.

With version 2 package has been renamed from "satooshi/php-coveralls"
to "php-coveralls/php-coveralls", and the script has been renamed
from "coveralls" to "php-coveralls"
@michalbundyra michalbundyra changed the title Updated travis configuration (slack, hhvm, php 7.2, locked) Updated travis configuration (slack, hhvm, php 7.2, locked, coveralls) Dec 14, 2017
@michalbundyra michalbundyra changed the title Updated travis configuration (slack, hhvm, php 7.2, locked, coveralls) Updated Travis CI configuration (slack, hhvm, php 7.2, locked, coveralls) Dec 14, 2017
@michalbundyra michalbundyra changed the title Updated Travis CI configuration (slack, hhvm, php 7.2, locked, coveralls) Updated Travis CI configuration (slack, hhvm, php 7.2, locked, coveralls) + docs rebase Apr 12, 2018
@weierophinney weierophinney merged commit e5097fc into zendframework:master Apr 25, 2018
PHP 7.2 automation moved this from To Do to Done Apr 25, 2018
weierophinney added a commit that referenced this pull request Apr 25, 2018
weierophinney added a commit that referenced this pull request Apr 25, 2018
Forward port #78
Forward port #91

Conflicts:
	CHANGELOG.md
weierophinney added a commit that referenced this pull request Apr 25, 2018
@weierophinney
Copy link
Member

Thanks, @webimpress

@michalbundyra michalbundyra deleted the hotfix/travis-configuration branch April 25, 2018 21:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
PHP 7.2
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants