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

HHVM was removed. Remove from travis conditionals #16281

Merged
merged 4 commits into from
May 27, 2017

Conversation

photodude
Copy link
Contributor

@photodude photodude commented May 26, 2017

Pull Request for Issue complete removal started with #16232

Summary of Changes

removes some travis items that were specific to HHVM

Testing Instructions

review

Expected result

unit tests pass

Actual result

unit tests pass

Documentation Changes Required

HHVM might work but is unsupported and no longer tested (add notice in requirements if not existing)

Additional information

in addition to passing our unit tests, we did have one user who did local testing on hhvm after the unit test were passing

Basically, Joomla CMS may work on HHVM but not supported. (kind of like mariaDB and other mysql variants)

There are 2 major reasons that Laravel, Symfony, Doctrine, CakePHP, MongoDB, PHPUnit, Composer, Twig, Silex, and Swiftmailer, etc have all dropped support for HHVM (that is something like 75% of the php market use has dropped support). 1) HHVM has needed to fix some big compatibility issues and 2) HHVM needs to improve their support when library devs report issues. With most of the big libraries moving to PHP7 only issues like HHVM and PHP7 differ on type annotations on internal functions (rtrim in particular) hhvm 7198 block the ability to test HHVM in HHVM's PHP7 mode since composer is unable to function due to that bug. Additionally, Cpanel and the like have never placed HHVM into the standard hosting options with PHP; resulting in limited user adoption with only places like Etsy on VMs or dedicated servers using HHVM. Add to that Travis testing of HHVM has tended to be outdated or slow and the new trusty container on travis is broken with mysql; testing is often the first step to support.

Personally, I hope that HHVM gets their act together so we and others can consider supporting HHVM again but at the moment it's a lame duck with little progress for those not programming with hiphop.

@photodude
Copy link
Contributor Author

/CC @rdeutz

if [[ $INSTALL_REDIS == "yes" && $TRAVIS_PHP_VERSION != hhvm ]]; then phpenv config-add "$BASE/build/travis/phpenv/redis.ini"; fi
if [[ $INSTALL_REDIS == "yes" && $TRAVIS_PHP_VERSION = hhvm ]]; then cat "$BASE/build/travis/phpenv/redis.ini" >> /etc/hhvm/php.ini; fi
if [[ $INSTALL_REDIS == "yes" ]]; then phpenv config-add "$BASE/build/travis/phpenv/redis.ini"; fi
if [[ $INSTALL_REDIS == "yes" ]]; then cat "$BASE/build/travis/phpenv/redis.ini" >> /etc/hhvm/php.ini; fi
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove /etc/hhvm/php.ini?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also lines 32 and 33 have the same tests. Combine them?

Copy link
Contributor

Choose a reason for hiding this comment

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

both lines can be removed

Copy link
Contributor

Choose a reason for hiding this comment

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

ah only the second

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oversight should have been removed. completed now.

@Quy
Copy link
Contributor

Quy commented May 26, 2017

I have tested this item ✅ successfully on 7f3dab4

Code review.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16281.

@wilsonge wilsonge merged commit fade4cc into joomla:staging May 27, 2017
@photodude photodude deleted the patch-10 branch May 27, 2017 20:23
izharaazmi added a commit to izharaazmi/joomla-cms that referenced this pull request May 31, 2017
* staging: (1779 commits)
  Deprecate modules correction (joomla#16362)
  Removed unnecessary code in com_content (joomla#14628)
  Stop installation if minimum requirement isn't met. (joomla#15890)
  Deprecate parts of com_modules (joomla#16152)
  LICENSE.txt (joomla#16317)
  [a11y] [com_fields] icons in modals (joomla#15047)
  Load jQuery in associations edit layout (joomla#16240)
  HHVM was removed. Remove from travis conditionals (joomla#16281)
  Fix Stylesheet Mime type keeping b/c (joomla#16284)
  re-merge joomla#15068 and joomla#16256
  Add showon attribute to add mailto link parameter (joomla#16282)
  Fix notices on Contact form (joomla#16279)
  Updating dutch TinyMCE files
  Media upload form margin (joomla#16253)
  Changes to display atom feeds correctly (joomla#16105)
  admin mod_latest (joomla#16277)
  Add truncate class and implement in popular articles module (joomla#16257)
  Using the "Special:MyLanguage" tag for links pointing to docs.joomla.org (joomla#15858)
  Change email notification to use site from address (Fix joomla#9261) (joomla#13518)
  Fix double encode ampersand in contact select list value (joomla#16268)
  ...
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.

6 participants