Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1523
THRIFT-4537
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1523
Thanks for checking, we need to stabilize these CI builds so less effort is
spent figuring out if a change is at least somewhat ok.
---
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
---
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1522#discussion_r177720513
--- Diff: lib/java/src/org/apache/thrift/annotations/Nullable.java ---
@@ -0,0 +1,33 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF
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 user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1518
Closing, will re-open once the dist jobs are working.
---
Github user jeking3 closed the pull request at:
https://github.com/apache/thrift/pull/1518
---
GitHub user jeking3 opened a pull request:
https://github.com/apache/thrift/pull/1518
THRIFT-4342: update ruby tests to use rspec 3, updated all dependencies for
ruby
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jeking3
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1515
The new style changes and checks are pretty recent, based on work done by
@RobberPhex.
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1515
Fortunately these changes are confined to one file so reviewing it isn't
too bad - hopefully they pass the new php formatting tests in the sca build.
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1515
Please squash your changes to a single commit if possible. Thanks.
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1515
THRIFT-4171
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1491
Looks good, will merge in the morning.
---
GitHub user jeking3 opened a pull request:
https://github.com/apache/thrift/pull/1514
THRIFT-4525: add ruby cross test ssl support
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jeking3/thrift THRIFT-4525
Alternatively you can
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1269
Remember to resolve https://issues.apache.org/jira/browse/THRIFT-4187 as
fixed in 0.12.0.
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1491
This squash was not done properly. It includes all of the master commits
in it.
You want to start with an updated master:
git checkout master
git pull
Then you want
GitHub user jeking3 opened a pull request:
https://github.com/apache/thrift/pull/1513
THRIFT-4358: add unix domain socket option to ruby cross tests
This is the first time I've tried to use ruby with any seriousness. I like
the unit test framework (rspec) -
very descriptive
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1512
Build failures appear unrelated to the change.
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1511
Thanks for the squash. We'll get this merged.
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1084
Okay, so I could merge this, but I need to know (since I've never used
cocoa, swift, or any of the apple stuff really):
* Are there breaking changes?
* Are they documented
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/972
@jfarrell please close this; the author indicated no interest in finishing
the work due to the team's past inaction on pull request management.
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/798
I am okay making this change for 0.12.0; if you can rebase, squash, and fix
up the issues and force push, and if there are any other methods that need the
same const correctness, let's do those too.
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/782
Should this be moved forward or closed?
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/744
@lihanharry please move this forward or it will eventually be closed due to
inactivity. @RobberPhex any concerns with this code?
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1269
I'd suggest waiting to see how the CI build fares given a change was made.
So today is a distinct possibility. Also, you can commit your own changes.
There's no rule against it. :)
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1491
Okay so, here's the deal. This pull request has 9 commits and one of them
is a merge. If you want your name on the commit tag you will need to squash
and resubmit with a single commit and then I
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1491
I get an error that may be unrelated when I run cross tests on --server
nodejs locally. This one test failes sporadically and I don't know if it could
be related to these changes; the client has
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1491
I am running a cross test on this locally; if it passes I will merge it, so
don't take any action.
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1289
@nsuke there must be some common wisdom and/or tooling to facilitate the
conversion from py2 to py3 in this regard - what have other projects done when
facing this issue?
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1496
Yes, however, it needs to be squashed to a single commit. I would prefer
if you did that, but I can do it if you really need me to. It's just a lot
more work for the committers to do that, and we
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1491
Perhaps; my PR for THRIFT-4515 improves the stability and coverage of
crosstest by adding more controlled server shutdown, and it passed tests (I've
run it many times on my local fork as well). I
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1509
The build failure in the artful check job is in lib/d unit tests and is a
sporadic issue that needs to be investigated, but will not hold up this work.
---
GitHub user jeking3 opened a pull request:
https://github.com/apache/thrift/pull/1509
THRIFT-4515: cross server test improvement: graceful test server shutdown
### Problem ###
Test servers are not shut down gracefully - they are SIGKILLed once the
client exits
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1496#discussion_r175276394
--- Diff: build/appveyor/MSVC-appveyor-build.bat ---
@@ -39,7 +39,7 @@ CD "%BUILDDIR%" || EXIT /B
-DW
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1496#discussion_r175276410
--- Diff: compiler/cpp/src/thrift/generate/t_generator.h ---
@@ -268,6 +271,30 @@ class t_generator {
return out.str();
}
+ const
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1496
If there's a way to fix it in the compiler so that it works properly,
that's much better. Having compiler-version-specific branches in the thrift
compiler to handle floating point oddities like
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1507
Go ahead and merge. After you merge, I update my fork and kick a build on
my account to refresh the images. It's an optimization that saves ~10 minutes
per build job.
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1496
You may be missing my point here. Regardless of the compiler used to build
the thrift compiler, whether it is gcc-4.6, clang-6.0, MSVC 2010, MSVC 2013,
MSVC 2017, the floating constants emitted
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1507
I'll keep an eye on this. Right now the docker images on docker.io have to
be generated manually every time they are updated, otherwise the build jobs
take more time than they should.
---
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1507#discussion_r174810986
--- Diff: lib/go/test/dontexportrwtest/compile_test.go ---
@@ -29,10 +28,10 @@ import (
func TestReadWriteMethodsArePrivate(t *testing.T
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1496
Need to squash to one commit, ideally, to make a merge easier, and fix the
issue causing older MSVC compilers (which people still use...) to generate
different floating constants than other
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1496#discussion_r174771623
--- Diff: test/DoubleConstantsTest.thrift ---
@@ -0,0 +1,17 @@
+namespace java thrift.test
+namespace cpp thrift.test
+
+// more tests
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1491
Please rebase on master one more time, most of the build issues are gone
now.
---
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1496#discussion_r174495089
--- Diff: test/DoubleConstantsTest.thrift ---
@@ -0,0 +1,17 @@
+namespace java thrift.test
+namespace cpp thrift.test
+
+// more tests
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1496#discussion_r174494320
--- Diff: lib/java/gradle/unitTests.gradle ---
@@ -68,6 +68,20 @@ test {
include '**/Test*.class'
exclude '**/Test*\$*.class
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1496
Thanks for dealing with the relatively unstable CI. Our project has
support for 25 languages, so needless to say without a fixed build environment
(which is impossible, since packages change over
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
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1474
@jfarrell can you please give a final say on this?
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1480
@bananer apart from missing any testing, is this a reasonable enhancement?
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1058
In general I want to make sure all clients send a seqid and all servers put
that seqid in the result. Without this we won't be able to have clients that
can have multiple outstanding requests
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1269
This needs to be rebased and completed.
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1496
I put the MSVC2013 job back in to make sure we had some backwards
compatibility... glad I did. I wonder if we have 32-bit issues with double in
general in thrift?
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1380
THRIFT-3458 is also related, and the approach in THRIFT-3458 would allow us
to add the apache git repo to dub directly so we should move in that direction,
but still try to use dub to build in lib
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1447
@lompy will you be able to carry this forward?
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1479
Any other php folks out there want to comment or review on the breaking
change here? It looks like the strategy was to change the generator and
provide a flag to allow folks to generate older
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1261
If you are able to finish this up and do the tutorial code, or if you are
not able to complete this, please let me know.
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1141
@bananer this would be a good candidate to pick up and run with.
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1058
@bananer is this worth pulling forward if it passes a build?
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1075
@bananer is this worth pulling forward if it passes a build? Is it a
breaking change?
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1506
With this merged would you say that THRIFT-4509 is now fixed, or is there
more work to do?
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1506
I see, you need to build Java first?
---
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 user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1502
In the future you can squash, rebase, and force push to keep the same PR,
see:
https://thrift.apache.org/docs/HowToContribute
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1502
Okay that sounds good; could you squash and rebase on master to prepare for
a merge?
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1496
Appveyor: It looks like I broke the build yesterday with a check-in that I
did not run through CI. I am fixing it.
Travis: c_glib, that's new; npm; that might be an intermittent failure
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1502
Any special considerations we need to add to the README for js or nodejs if
we check in a package-lock.json file? When distributing through npm should
this file also be distributed?
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1486
Hmm, looks like I broke the build right before this; I'll make sure this
carries through and merge it. Thanks.
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1497
Cool so that last test run was without the fix (I thought it had it, but it
only had my fixes to TestServer.cpp), and adding your fix seems to have taken
care of the header failures somehow
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1497
You need to create a Jira ticket in the THRIFT project
(https://issues.apache.org/jira/projects/THRIFT/issues) that describes the bug
and resolution so that I can merge it.
---
I
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1497
What's the Jira ticket ID for this work?
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1497
Wow, haven't seen a green build in a while. Of course, none of the cross
tests exercise nonblocking code. :( Would be nice to add this as another
matrix option.
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1448
I tried to use this work and did a little bit of cmake work to straighten
out options. What I found is that there is a boost dependency in the
safe_numeric_cast in TTransportException.hpp
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1448#discussion_r173620082
--- Diff: build/cmake/DefineOptions.cmake ---
@@ -91,11 +91,26 @@ if(WITH_CPP)
option(WITH_STDTHREADS "Build with C++ std::thread support
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1474
@jfarrell can you please give a final say on this?
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1497
Rebase on master and let's see what happens, thanks.
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1496
The current master is fairly clean except for a recurring dlang issue.
Squash and rebase on master. Squash to one commit please.
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1479
Document all breaking changes in the language README.
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1480
This needs to be retargeted at the master branch and it needs an Apache
Jira thrift ticket. See https://thrift.apache.org/docs/HowToContribute
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1494
Ugh, build failure is dlang related, not to this.
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1494
I'm running this and 4497 through local tests since it wasn't based on good
CI builds.
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1504
Usually we ask for a Jira ticket for each change but I'm not going to
considering this is a small doc change. In the future please see:
https://thrift.apache.org/docs/HowToContribute
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1500
As of yesterday afternoon 4.x is no longer used.
Due to the dependencies, and due to the fact that node.js 4.x LTS ends next
month, I moved the "oldest" make check job to use
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1497#discussion_r172695014
--- Diff: lib/cpp/src/thrift/server/TNonblockingServer.cpp ---
@@ -472,6 +472,18 @@ void TNonblockingServer::TConnection::workSocket
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1497#discussion_r172694884
--- Diff: lib/cpp/src/thrift/server/TNonblockingServer.cpp ---
@@ -472,6 +472,18 @@ void TNonblockingServer::TConnection::workSocket
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1495
Hi, please rebase this on the current master and force push to kick a new
build - I think the CI build environment is stable again.
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1494
Hi, please rebase this on the current master and force push to kick a new
build - I think the CI build environment is stable again.
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1491
If you haven't seen it I recommend reading `build/docker/README.md` and use
the `ubuntu-artful` image to run a cross test with
`build/docker/scripts/cross-test.sh` inside the docker container
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1491
The presence of "--domain-socket" to the client or server implies uds.
This needs to be fixed, and domain needs to be moved from haskell to nodejs,
and the cross tests that are fai
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1491#discussion_r172557523
--- Diff: test/tests.json ---
@@ -221,7 +221,8 @@
"framed"
],
"sockets": [
- "ip"
+
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1496
Sourceforge was down for a while. I am working on getting the builds back
to green (passing). I recommend that you keep your pull request squashed to
one commit. In fact since you are doing
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1269
This needs to be rebased and completed.
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1380
The tutorials didn't get updated and as a result fail to build in the
autotools build (build/docket/script/autotools.sh) which is what some of the
Travis CI build jobs do. I am attempting to fix this.
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1448
Thanks, that's incredibly helpful when it comes to merging it.
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1491
Where does the "--type uds" option get passed into the python test client
or server in order to convince it to use domain sockets? In other test suites
like cpp, only "--domain_sock
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1499
THRIFT-4505
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1499
Hi, in the future please follow the instructions at
https://thrift.apache.org/docs/HowToContribute, particularly opening a Jira
ticket and tracking the pull request against it. I will do
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1380
Opened THRIFT-4504 to track this.
---
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1380
I'd like to see this squashed to one commit and rebased one more time; our
CI builds are not in good shape but I can run it on my local system to prove it
out and they deal with CI issues when
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1474
Based on my read of the file from which the original source information
came, the original fbthrift project files are Apache Source licensed. So I'm
not certain we need to do anything
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1412
I updated the ubuntu-artful image to SBCL 1.4.4 and it seems to be stable.
Merging.
---
1 - 100 of 1222 matches
Mail list logo
