You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by romanoid <gi...@git.apache.org> on 2018/03/09 18:27:18 UTC
[GitHub] thrift pull request #1505: Fixing java thrift compiler to generate constants...
GitHub user romanoid opened a pull request:
https://github.com/apache/thrift/pull/1505
Fixing java thrift compiler to generate constants in stable order.
For description of the issue fixed, see: https://issues.apache.org/jira/browse/THRIFT-4513
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/romanoid/thrift fix-constant-order
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/thrift/pull/1505.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1505
----
commit 6a280b458f95a24d20bbc7798e145fe703ed23a0
Author: Roman Soroka <ro...@...>
Date: 2018-03-08T23:45:22Z
Fixing java thrift compiler to generate constants in stable order.
See https://issues.apache.org/jira/browse/THRIFT-4513
----
---
[GitHub] thrift issue #1505: THRIFT-4513: Fixing java thrift compiler to generate con...
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1505
Please squash and rebase on master, I just merged 4 commits that stabilize the CI builds.
---
[GitHub] thrift issue #1505: THRIFT-4513: Fixing java thrift compiler to generate con...
Posted by romanoid <gi...@git.apache.org>.
Github user romanoid commented on the issue:
https://github.com/apache/thrift/pull/1505
@jeking3 Yes, thanks, PR had some assumption on Map keys being only primitive for constant maps, which is not true in general. Latest revision should no longer rely on it.
---
[GitHub] thrift issue #1505: THRIFT-4513: Fixing java thrift compiler to generate con...
Posted by romanoid <gi...@git.apache.org>.
Github user romanoid commented on the issue:
https://github.com/apache/thrift/pull/1505
@jeking3 done, thanks, let's see if it passes.
---
[GitHub] thrift pull request #1505: THRIFT-4513: Fixing java thrift compiler to gener...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/thrift/pull/1505
---
[GitHub] thrift issue #1505: Fixing java thrift compiler to generate constants in sta...
Posted by dcelasun <gi...@git.apache.org>.
Github user dcelasun commented on the issue:
https://github.com/apache/thrift/pull/1505
Please add the issue number to your commit message and PR title. See [here](https://thrift.apache.org/docs/HowToContribute) (point 7 under Github).
---
[GitHub] thrift issue #1505: THRIFT-4513: Fixing java thrift compiler to generate con...
Posted by romanoid <gi...@git.apache.org>.
Github user romanoid commented on the issue:
https://github.com/apache/thrift/pull/1505
Updated approach:
t_const_value now defines ordering.
Java generator changed to delegate to native ordering of t_const_value
---
[GitHub] thrift issue #1505: THRIFT-4513: Fixing java thrift compiler to generate con...
Posted by romanoid <gi...@git.apache.org>.
Github user romanoid commented on the issue:
https://github.com/apache/thrift/pull/1505
Thank you for the review!
---
[GitHub] thrift issue #1505: THRIFT-4513: Fixing java thrift compiler to generate con...
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1505
Looks like a dlang issue that pops up from time to time. Just to satisfy myself this change is okay I need to look at generated thrift file comparisons from before and after this change. I'll do that locally.
---
[GitHub] thrift issue #1505: THRIFT-4513: Fixing java thrift compiler to generate con...
Posted by romanoid <gi...@git.apache.org>.
Github user romanoid commented on the issue:
https://github.com/apache/thrift/pull/1505
Fix is in, the only failure left is this one, I'm not sure if it is instability of tests or real problem: https://travis-ci.org/apache/thrift/jobs/353050154
---