You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by allengeorge <gi...@git.apache.org> on 2016/12/11 13:36:24 UTC
[GitHub] thrift pull request #1138: Add union example to ThriftTest
GitHub user allengeorge opened a pull request:
https://github.com/apache/thrift/pull/1138
Add union example to ThriftTest
This simply adds a union definition to ThriftTest.thrift.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/allengeorge/thrift thrift_union_in_thrift_test
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/thrift/pull/1138.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 #1138
----
commit 2dc5ba5c43b3f2cc164fa27e9378379a9f6924cb
Author: Allen George <al...@gmail.com>
Date: 2016-12-11T13:33:52Z
Add union example to ThriftTest
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request #1138: Add union example to ThriftTest
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/thrift/pull/1138
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift issue #1138: Add union example to ThriftTest
Posted by mrwiggles <gi...@git.apache.org>.
Github user mrwiggles commented on the issue:
https://github.com/apache/thrift/pull/1138
Thanks @Jens-G
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift issue #1138: Add union example to ThriftTest
Posted by Jens-G <gi...@git.apache.org>.
Github user Jens-G commented on the issue:
https://github.com/apache/thrift/pull/1138
The calls are documented in ThiftTest.thrift. I'd suggest to follow the model we have. Maybe a bit more verbose. What should become claer is the expected inputs and the expected returns. Needed in order to have all server/client pairs run successfully during cross tests or the like.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift issue #1138: Add union example to ThriftTest
Posted by jfarrell <gi...@git.apache.org>.
Github user jfarrell commented on the issue:
https://github.com/apache/thrift/pull/1138
I just rebased the release candidate against master and fixed the broken haskel tests so if parts of this are committed all ready then they will be apart of the release candidate. I would advocate for committing as portions become available and not hiding in a branch or personal fork and try and structure the commits as logical groups which do not break anything as they go in
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift issue #1138: Add union example to ThriftTest
Posted by Jens-G <gi...@git.apache.org>.
Github user Jens-G commented on the issue:
https://github.com/apache/thrift/pull/1138
But adding a service function will break things?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift issue #1138: Add union example to ThriftTest
Posted by jfarrell <gi...@git.apache.org>.
Github user jfarrell commented on the issue:
https://github.com/apache/thrift/pull/1138
could use a stub with a successful return and a TODO comment
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift issue #1138: Add union example to ThriftTest
Posted by allengeorge <gi...@git.apache.org>.
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1138
Ok. Should I put notes in the ticket as to what would make a good candidate service call?
On Mon, Dec 12, 2016 at 9:07 PM -0500, "jfarrell" <no...@github.com> wrote:
could use a stub with a successful return and a TODO comment
\u2014
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift issue #1138: Add union example to ThriftTest
Posted by Jens-G <gi...@git.apache.org>.
Github user Jens-G commented on the issue:
https://github.com/apache/thrift/pull/1138
Correct. I merged it, but left the ticket open. The idea is to have it act as an umbrella ticket to cover all languages after some suitable call has been added to the service and a test definition has been formulated.
Since this will break the cross test until all relevant languages are integrated we should either have a feature branch. I could do that and coordinate the patches via my personal Thrift fork, then finally merge the whole feature branch back into Apache Thrift master.
@jfarrell Does that sound like a plan to you?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift issue #1138: Add union example to ThriftTest
Posted by allengeorge <gi...@git.apache.org>.
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1138
Do I have to create a ticket number for this?
Also, I decided to separate simply *defining* the union from actually using it in a service call.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift issue #1138: Add union example to ThriftTest
Posted by allengeorge <gi...@git.apache.org>.
Github user allengeorge commented on the issue:
https://github.com/apache/thrift/pull/1138
If I understand correctly - this is merged now? If so - thanks @Jens-G, and I'll get the second half of the patch in later this week. Sorry about the slow turnaround time.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift issue #1138: Add union example to ThriftTest
Posted by Jens-G <gi...@git.apache.org>.
Github user Jens-G commented on the issue:
https://github.com/apache/thrift/pull/1138
THRIFT-4000
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---