Github user dtmuller commented on the issue:
https://github.com/apache/thrift/pull/1039
@nsuke Thanks a lot for your effort - I really enjoyed working with 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
Github user nsuke commented on the issue:
https://github.com/apache/thrift/pull/1039
@dtmuller just committed, thanks for all the help and improvements !
It might have not happened at all if you didn't pick it up and revive it ...
---
If your project is set up for it, you can
Github user dtmuller commented on the issue:
https://github.com/apache/thrift/pull/1039
@nsuke That's great, thanks. I see, the two PRs are in sync, so I will
close this one...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as
Github user nsuke commented on the issue:
https://github.com/apache/thrift/pull/1039
@dtmuller as you may have noticed I've squashed commits for merging in #368.
If there's no concern I'll push them within a week or so.
---
If your project is set up for it, you can reply to this
Github user dtmuller commented on the issue:
https://github.com/apache/thrift/pull/1039
@nsuke Do you have a rough idea when this PR can be merged? If there is
more work to do I can assist, please let me know...
---
If your project is set up for it, you can reply to this email and
Github user dtmuller commented on the issue:
https://github.com/apache/thrift/pull/1039
So, it looks like this could make it into the master soon - once CI is
green. :smiley:
Unfortunately I'm afraid that I can't help with the windows fix.
---
If your project is set up for it,
Github user nsuke commented on the issue:
https://github.com/apache/thrift/pull/1039
The `thrift` directory makes sense.
The test # 4 was relying on `sleep` in test script that is inherently flaky.
I've removed a double pclose in parent process code so we'll see if it
fixes
Github user dtmuller commented on the issue:
https://github.com/apache/thrift/pull/1039
I added the deployment of headers needed to build plugins. I changed the
directory structure to look the same as under /lib (added an extra thrift
folder). The reason: for cmake I batch-copied the
Github user dtmuller commented on the issue:
https://github.com/apache/thrift/pull/1039
When plugins are enabled, we should also deploy the necessary headers. I
will tried to do this, but I'm having a hard time with Autotools at the
moment. Need a few more days to fix this...
---
Github user nsuke commented on the issue:
https://github.com/apache/thrift/pull/1039
@dtmuller thanks, the new changes look good.
As you uncovered another defect on includes, I've devised simple
integration test using `diff` and resolved a few issues too.
I've pushed some
Github user dtmuller commented on the issue:
https://github.com/apache/thrift/pull/1039
I fixed a few of your findings. On the remaining two issues concerning
`t_audit/t_logging` and the shared boost test support I'm not sure what to do...
---
If your project is set up for it, you
Github user nsuke commented on the issue:
https://github.com/apache/thrift/pull/1039
This looks mostly good except for a few issues I commented.
The t_audit part seems to be the biggest challenge.
Not sure you're going to work through them, as some of them are from
original
Github user nsuke commented on the issue:
https://github.com/apache/thrift/pull/1039
@dtmuller thanks ! I'll look into the weekend after next.
---
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
13 matches
Mail list logo
