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

Test with PHP 7.4 #819

Closed
oliverklee opened this issue Oct 30, 2019 · 12 comments · Fixed by #821
Closed

Test with PHP 7.4 #819

oliverklee opened this issue Oct 30, 2019 · 12 comments · Fixed by #821

Comments

@oliverklee
Copy link
Contributor

We probably need to disable the platform requirements constrains for Composer for this.

@opengeek
Copy link

opengeek commented Dec 3, 2019

Is this going to get done soon? This issue is blocking the composer install of ALL projects which depend on this package for no good reason that I can see. The problem is resolved by merely changing the PHP platform requirements to use >=5.6 instead of specifying each version. Is there any way this could be fast-tracked into the current major releases?

@oliverklee
Copy link
Contributor Author

@opengeek We could do a feature release 3.1.0 for this. Would you be willing to create a pull request for adding 7.4 support and adding 7.4 to Travis CI?

oliverklee added a commit that referenced this issue Dec 3, 2019
oliverklee added a commit that referenced this issue Dec 3, 2019
oliverklee added a commit that referenced this issue Dec 3, 2019
oliverklee added a commit that referenced this issue Dec 3, 2019
oliverklee added a commit that referenced this issue Dec 3, 2019
oliverklee added a commit that referenced this issue Dec 3, 2019
oliverklee added a commit that referenced this issue Dec 3, 2019
@burzum
Copy link

burzum commented Dec 5, 2019

@oliverklee when can we expect a release for this?

@oliverklee
Copy link
Contributor Author

Probably sometime next week depending on when I can put the time into this.
If you want to help to speed things up (which I would greatly appreciate): #821 needs a condition in the .travis.yml to only execute Psalm on PHP < 7.4.

@burzum
Copy link

burzum commented Dec 5, 2019

@oliverklee OK, but the issue doesn't seem to be Psalm but something with phpunit? https://travis-ci.org/MyIntervals/emogrifier/jobs/620305033?utm_medium=notification&utm_source=github_status

@oliverklee
Copy link
Contributor Author

@burzum It's like this:

  • We currently are using PHPUnit 6.5 and Psalm 3.2 to be able to run the tests on PHP 7.0 as well (which is the lowest PHP version Emogrifier currently supports).
  • Psalm 3.2 has bugs that prevent it from running under PHP 7.4
  • Psalm 3.7 requires PHP >= 7.1, and is also has dependencies that prevent if from being installed alongside with PHPUnit 6.5
  • In addition, Psalm 3.7 has different rules than Psalm 3.2, which would require changes to our codebase (which I'd like to avoid for a short-term release to support PHP 7.4).

So the idea is to skip the Psalm checks on Travis CI on PHP >= 7.4. And when we raise our minimum version requirement for PHP to 7.1, we can update PHPUnit and Psalm and also update our code for the latest Psalm.

@oliverklee
Copy link
Contributor Author

(And I would not be comfortable declaring compatibility with unbounded PHP >= 7.0 version requirement without having our tests run on the respective PHP versions.)

@burzum
Copy link

burzum commented Dec 5, 2019

I've tried to reproduce what I see on travis locally but it just works for me without the issues I see on travis. This is what it runs, no?

composer remove --dev vimeo/psalm
composer update --prefer-lowest

@oliverklee
Copy link
Contributor Author

To avoid these kinds of dependency problems in the long run, we should not require the CI tools as Composer dev dependencies, but have them as PHARs using Phive the way https://github.com/sebastianbergmann/phpunit/ does it.

@oliverklee
Copy link
Contributor Author

oliverklee commented Dec 5, 2019

@burzum Yes, and also composer ci:psalm is executed (which is where the problem occurs). This is what we need to skip on PHP 7.4. That's the only thing needed short-term to get the 7.4 build to pass AFAICS.

@burzum
Copy link

burzum commented Dec 5, 2019

@oliverklee OK, I'll give it a try, no promise but I'll try it. I have a surgery day today and see what I can get done after that or tomorrow, depending on whats going on at work.

@oliverklee
Copy link
Contributor Author

@burzum As this issue is about getting support for PHP 7.4, I've moved your comment to the ticket #822 that is about using Phive.

@MyIntervals MyIntervals deleted a comment from burzum Dec 5, 2019
oliverklee added a commit that referenced this issue Dec 25, 2019
Also only run the CI tasks that do not depend on the PHP version
(i.e., everything except for the PHP linting and the unit tests)
on PHP 7.3 only to avoid problems with PHP 7.4.

Also ignore the platform reqs for PHP 7.3 and 7.4 during composer update
for the time being.

Fixes #819
oliverklee added a commit that referenced this issue Dec 25, 2019
Also only run the CI tasks that do not depend on the PHP version
(i.e., everything except for the PHP linting and the unit tests)
on PHP 7.3 only to avoid problems with PHP 7.4.

Also ignore the platform reqs for PHP 7.3 and 7.4 during composer update
for the time being.

Fixes #819
oliverklee added a commit that referenced this issue Dec 25, 2019
Also only run the CI tasks that do not depend on the PHP version
(i.e., everything except for the PHP linting and the unit tests)
on PHP 7.3 only to avoid problems with PHP 7.4.

Also ignore the platform reqs for PHP 7.3 and 7.4 during composer update
for the time being.

Fixes #819
oliverklee added a commit that referenced this issue Dec 25, 2019
Also only run the CI tasks that do not depend on the PHP version
(i.e., everything except for the PHP linting and the unit tests)
on PHP 7.3 only to avoid problems with PHP 7.4.

Also ignore the platform reqs for PHP 7.3 and 7.4 during composer update
for the time being.

Fixes #819
oliverklee added a commit that referenced this issue Dec 25, 2019
Also only run the CI tasks that do not depend on the PHP version
(i.e., everything except for the PHP linting and the unit tests)
on PHP 7.3 only to avoid problems with PHP 7.4.

Also ignore the platform reqs for PHP 7.3 and 7.4 during composer update
for the time being.

Fixes #819
oliverklee added a commit that referenced this issue Dec 25, 2019
Also only run the CI tasks that do not depend on the PHP version
(i.e., everything except for the PHP linting and the unit tests)
on PHP 7.3 only to avoid problems with PHP 7.4.

Also ignore the platform reqs for PHP 7.3 and 7.4 during composer update
for the time being.

Fixes #819
oliverklee added a commit that referenced this issue Dec 25, 2019
Also only run the CI tasks that do not depend on the PHP version
(i.e., everything except for the PHP linting and the unit tests)
on PHP 7.3 only to avoid problems with PHP 7.4.

Also ignore the platform reqs for PHP 7.3 and 7.4 during composer update
for the time being.

Fixes #819
zoliszabo pushed a commit that referenced this issue Dec 26, 2019
Also only run the CI tasks that do not depend on the PHP version
(i.e., everything except for the PHP linting and the unit tests)
on PHP 7.3 only to avoid problems with PHP 7.4.

Also ignore the platform reqs for PHP 7.3 and 7.4 during composer update
for the time being.

Fixes #819
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants