search

reeze / msgpack-hhvm

Msgpack for HHVM (msgpack.org[HHVM])
MIT License
14 stars 16 forks source link

Building against HHVM 3.12.0 #10

Closed gui-com-pt closed 8 years ago

gui-com-pt commented 8 years ago

With I only renamed the function KindOfStaticString as stated in this commit: facebook/hhvm@127a039edfed7558bb3524cf02a42a0da0a3f7fc

I've sent a pull request at reeze/msgpack-hhvm#9

reeze commented 8 years ago

Hi @guilhermegeek

thanks for the PR could you please guard it with version check to make it compatible with the old version?

gui-com-pt commented 8 years ago

Shouldn't you use a new branch for 3.12 instead, and recommend users higher than 3.X to use it? By guard it you mean comparing the HHVM_VERSION constant right? The switch statement would be duplicated (almost 50 lines of code). Sorry but i got confused with the "guard" word, it's my first PR of this type ;)

reeze commented 8 years ago

Hi,

Yes. I mean maybe we could define KindOfStaticString as an macro and define it to KindOfPersistentString if the version is larger than the latest renamed version.

maybe something like this: 

# if HHVM_VERSION <  xxx
# define KindOfStaticString KindOfStaticString
# else
# define KindOfStaticString KindOfPersistentString
#endif
gui-com-pt commented 8 years ago

Hi,

Thank you for the guidance! I've used HHVM_VERSION_ID and the build has passed. I had rebased 3 commits i made (the reason where there're 3 builds in Travis).

reeze commented 8 years ago

Great, Thanks for your work!