[GitHub] thrift issue #1522: THRIFT-4530: add @Nullable annotations to generated Java...

2018-03-27 Thread msridhar
Github user msridhar commented on the issue:

https://github.com/apache/thrift/pull/1522
  
Note that while there are no new tests, the code under 
`tutorial/java/gen-java` includes the annotations and compiles without issue.  
Since this change doesn't affect runtime behavior of the generated code I think 
this might be sufficient.


---


[GitHub] thrift issue #1522: THRIFT-4530: add @Nullable annotations to generated Java...

2018-03-27 Thread msridhar
Github user msridhar commented on the issue:

https://github.com/apache/thrift/pull/1522
  
Looks like the Travis failure is on Common Lisp codegen 
(https://api.travis-ci.org/v3/job/359136811/log.txt), which has nothing to do 
with this PR


---


[GitHub] thrift issue #1522: THRIFT-4530: add @Nullable annotations to generated Java...

2018-03-27 Thread jeking3
Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1522
  
Yes there is an issue that appears in builds where lisp cannot find a file 
it just supposedly wrote.  Lisp is new to the project, this is an ongoing thing.


---


[GitHub] thrift issue #1522: THRIFT-4530: add @Nullable annotations to generated Java...

2018-03-28 Thread msridhar
Github user msridhar commented on the issue:

https://github.com/apache/thrift/pull/1522
  
I just pushed up another commit with the renaming.  I assume you can do a 
squash and merge, but if you'd like me to squash just let me know.


---


[GitHub] thrift issue #1522: THRIFT-4530: add @Nullable annotations to generated Java...

2018-03-28 Thread dcelasun
Github user dcelasun commented on the issue:

https://github.com/apache/thrift/pull/1522
  
No need, I'll squash once Travis is all green.


---


[GitHub] thrift issue #1522: THRIFT-4530: add @Nullable annotations to generated Java...

2018-03-28 Thread msridhar
Github user msridhar commented on the issue:

https://github.com/apache/thrift/pull/1522
  
Hrm, now I see a concurrency test failure on appveyor.  Doesn't seem 
related to my change, though I'm not 100% sure.


---


[GitHub] thrift issue #1522: THRIFT-4530: add @Nullable annotations to generated Java...

2018-03-28 Thread dcelasun
Github user dcelasun commented on the issue:

https://github.com/apache/thrift/pull/1522
  
I don't think it's relevant either, seems like a flaky test. I think we are 
good to merge.

@jeking3 any idea on the failing appveyor test, have you seen it before? 
I'm inclined to ignore it unless you say otherwise.


---


[GitHub] thrift issue #1522: THRIFT-4530: add @Nullable annotations to generated Java...

2018-03-29 Thread jeking3
Github user jeking3 commented on the issue:

https://github.com/apache/thrift/pull/1522
  
That is an intermittent test failure that I thought I resolved.  :)  it is 
C++ only


---