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

Support hhvm 4.69+ #27

Merged
merged 2 commits into from
Aug 18, 2020
Merged

Support hhvm 4.69+ #27

merged 2 commits into from
Aug 18, 2020

Conversation

lexidor
Copy link
Collaborator

@lexidor lexidor commented Aug 15, 2020

Hhvm 4.62 introduced the requirements for whitelisting your hh_fixmes.
This didn't break for dependents, only when hammock is typechecked in isolation.

Hhvm 4.68 removed the array<_> and array<_, _> typehints.
Use varray<_> and darray<_, _> instead for maximum compatibility.
HEADS-UP: In older hhvm versions, (d/v)arrayness was not checked at runtime and never versions will error on typehint mismatches.

@lexidor
Copy link
Collaborator Author

lexidor commented Aug 15, 2020

@tyronjung-quizlet Travis is not picking up PRs for testing.
I'll test locally on hhvm 4.69, but I can't test older hhvm versions individually.
There are not that many changes to make, but when spanning over 100 weeks of hhvm versions, you can never be sure enough,

Adds hhconfig directives required for hhvm 4.62.
These directives are only needed when hammock is the main project.
If you use hammock as a dependency, you don't need this fix.
Fixes parse error in hhvm 4.68 for array<mixed>.
@tyronjung-quizlet
Copy link
Contributor

Hm weird, not sure why Travis isn't running...

Copy link
Contributor

@tyronjung-quizlet tyronjung-quizlet left a comment

Choose a reason for hiding this comment

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

Thanks for doing this! Travis isn't running but this change should be fine.

@tyronjung-quizlet tyronjung-quizlet merged commit cd369cc into quizlet:master Aug 18, 2020
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.

2 participants