[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 someone else who uses php 
regularly.  @Jens-G perhaps... to ensure this change is acceptable (removing 
php5 support).  I haven't seen any objections in the developer email list but 
it hasn't been expressly mentioned.


---


[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 already are some CI problems at php part.


---


[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 running any unit tests or anything that 
would parse (and fail, if it really is incompatible) the code?


---


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

2017-10-22 Thread RobberPhex
Github user RobberPhex commented on a diff in the pull request:

https://github.com/apache/thrift/pull/1391#discussion_r146125400
  
--- Diff: lib/php/src/ext/thrift_protocol/config.m4 ---
@@ -1,25 +1,27 @@
-dnl Copyright (C) 2009 Facebook
-dnl Copying and distribution of this file, with or without modification,
-dnl are permitted in any medium without royalty provided the copyright
-dnl notice and this notice are preserved.
+dnl Licensed to the Apache Software Foundation (ASF) under one
--- End diff --

OK, I just add APL declaration to `config.m4`


---