[GitHub] thrift issue #1391: Fix segment fault at thrift_protocol extension

2017-10-23 Thread RobberPhex
Github user RobberPhex commented on the issue: https://github.com/apache/thrift/pull/1391 @jeking3 Currently, I just remove PHP5 support at PHP extension(thrift_protocol), PHP library is still support PHP5. So, I think this PR is good. --- If PHP5

[GitHub] thrift issue #1391: Fix segment fault at thrift_protocol extension

2017-10-23 Thread jeking3
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1391 I posted a message on the developer mailing list about removing php5 support. We'll see what comes of it. php5 is currently still shipping so I have some reservations about removing support for

[GitHub] thrift issue #1391: Fix segment fault at thrift_protocol extension

2017-10-22 Thread jeking3
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1391 I already discussed the possibility of retiring the trusty build environment to reduce project maintenance. Maybw now is the time to do it. That said, I would really like to get input from

[GitHub] thrift issue #1391: Fix segment fault at thrift_protocol extension

2017-10-22 Thread RobberPhex
Github user RobberPhex commented on the issue: https://github.com/apache/thrift/pull/1391 at job `Autotools (Ubuntu Trusty)`, it just run `make check`, so php extension cannot be compiled. at job `Cross Language Tests`, it needn't php extension. So, I think there

[GitHub] thrift issue #1391: Fix segment fault at thrift_protocol extension

2017-10-22 Thread jeking3
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1391 Hmm, I'm just curious about this - given support for php5 was dropped, any idea why the ubuntu-trusty "make check" job succeeded (because it has PHP 5.5.9 in it)? Is the build job not actually

[GitHub] thrift issue #1391: Fix segment fault at thrift_protocol extension

2017-10-21 Thread RobberPhex
Github user RobberPhex commented on the issue: https://github.com/apache/thrift/pull/1391 Hi, about license. `Copyright (C) 2009 Facebook` at file `lib/php/src/ext/thrift_protocol/config.m4` was add by commit f82aee5087bd62989482f5c532cbd80f97a39b7f. But before this

[GitHub] thrift issue #1391: Fix segment fault at thrift_protocol extension

2017-10-21 Thread jeking3
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1391 @jfarrell I see licensing changes in this pull request, can you review? ---

[GitHub] thrift issue #1391: Fix segment fault at thrift_protocol extension

2017-10-20 Thread jeking3
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1391 I saw cross test pass before, so I think we're good. ---

[GitHub] thrift issue #1391: Fix segment fault at thrift_protocol extension

2017-10-20 Thread RobberPhex
Github user RobberPhex commented on the issue: https://github.com/apache/thrift/pull/1391 build job 4 failed due to https://issues.apache.org/jira/browse/THRIFT-2913. And, appveyor build queued. but, same commit at my repo CI is success:

[GitHub] thrift issue #1391: Fix segment fault at thrift_protocol extension

2017-10-20 Thread jeking3
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1391 UBsan build failed due to https://issues.apache.org/jira/browse/THRIFT-2913. ---

[GitHub] thrift issue #1391: Fix segment fault at thrift_protocol extension

2017-10-20 Thread RobberPhex
Github user RobberPhex commented on the issue: https://github.com/apache/thrift/pull/1391 this pr fix [`THRIFT-4356`](https://issues.apache.org/jira/browse/THRIFT-4356) and [`THRIFT-4353`](https://issues.apache.org/jira/browse/THRIFT-4353). --- And, this work is

[GitHub] thrift issue #1391: Fix segment fault at thrift_protocol extension

2017-10-20 Thread jeking3
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1391 Is this for THRIFT-4353? Can you squash it? Rebase it on master to fix the CI issues. ---