[GitHub] qpid-proton pull request: PROTON-881: Make connect a non-blocking ...
Github user rhs commented on the pull request: https://github.com/apache/qpid-proton/pull/40#issuecomment-118681852 FYI, I merged this onto trunk after landing the reactor branch. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] qpid-proton pull request: proton-c: minor code fixes
Github user rhs commented on the pull request: https://github.com/apache/qpid-proton/pull/26#issuecomment-99432258 Looks good to me. On the struct initialization stuff, it might be worth seeing if we can make gcc turn those into errors if it causes problems on other compilers. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] qpid-proton pull request: NO-JIRA: improvements to export.sh
Github user rhs commented on a diff in the pull request: https://github.com/apache/qpid-proton/pull/28#discussion_r29751867 --- Diff: bin/export.sh --- @@ -54,15 +65,18 @@ WORKDIR=$(mktemp -d) ## ( cd ${SRC} -TAG=$(git describe --tags --always) MTIME=$(date -d @`git log -1 --pretty=format:%ct tags/${TAG}` '+%Y-%m-%d %H:%M:%S') ARCHIVE=$DIR/qpid-proton-${TAG}.tar.gz +VERSION=$(git show tags/${TAG}:version.txt) +PREFIX=qpid-proton-${VERSION} [ -d ${WORKDIR} ] || mkdir -p ${WORKDIR} -git archive --format=tar --prefix=qpid-proton-${TAG}/ tags/${TAG} \ +git archive --format=tar --prefix=${PREFIX}/ tags/${TAG} \ | tar -x -C ${WORKDIR} +${SRC}/bin/version.sh ${WORKDIR}/${PREFIX} ${VERSION} --- End diff -- This doesn't make sense to me. This script is supposed to simply export exactly the contents of a tag in the git repo. This tag is generally a signed release, and running scripts over the content of the release will invalidate that process entirely. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] qpid-proton pull request: NO-JIRA: improvements to export.sh
Github user rhs commented on the pull request: https://github.com/apache/qpid-proton/pull/28#issuecomment-99428883 Fixing the prefix in the tarball makes sense to me, however some of the other changes do not. The release process is supposed to proceed from a signed tag, which means that if the version for that signed tag is wrong, then something prior in the process has gone wrong. In general having export.sh modify the contents of the exported source tree in any way is dangerous since it will lead to divergence between the contents of the git tag for a given release and the contents of the exported tarballs for that release, and that is obviously really bad. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] qpid-proton pull request: Reactor
Github user rhs commented on the pull request: https://github.com/apache/qpid-proton/pull/29#issuecomment-99635551 Given my recent rant about new development on master, let's try pulling this in on a branch first. I've create the proton-j-reactor branch. If you can resubmit against that I'll pull it in and raise the issue on the 0.10 release thread of whether we want to include it in the release. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] qpid-proton pull request: Fixes for Mac OS X
Github user rhs commented on the pull request: https://github.com/apache/qpid-proton/pull/23#issuecomment-97009507 Hmm, I'm not sure how useful the trace is since it is once where all the tests passed. Any chance you could grab one where the tests fail? Thinking a bit more, I think the test makes the assumption that the underlying loopback interface is synchronous, i.e. as soon as a write succeeds, any corresponding reads will be ready. I've added info to the assert that will hopefully test this theory. Can you do a git pull and reproduce the failure again and post the traceback? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] qpid-proton pull request: Fixes for Mac OS X
Github user rhs commented on the pull request: https://github.com/apache/qpid-proton/pull/23#issuecomment-97093665 For the record, I tried making the test assertion more specific, but it is difficult to come up with an assertion that accommodates the asynchronous behavior and isn't extremely brittle and specific. Given that I'm +1 on the change. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] qpid-proton pull request: Fixes for Mac OS X
Github user rhs commented on the pull request: https://github.com/apache/qpid-proton/pull/23#issuecomment-96734626 I'm a little puzzled as to why the test change would be necessary. Pump has been written to keep looping until there is no I/O left to perform for any of its messengers. Given that the receiving messenger has issued credit, and the sending messenger has posted messages for transfer, I think this is probably somehow indicative of a real bug somewhere (possibly in some of the test framework code). How easy is it to reproduce this on a mac? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] qpid-proton pull request: NO-JIRA: README improvements
Github user rhs commented on the pull request: https://github.com/apache/qpid-proton/pull/22#issuecomment-95922186 +1 As a general note I think it's good to try to test changes to install/readme type stuff on people who aren't already intimately familiar with proton and all its various quirks. I wouldn't hold up merging this for that though. Nice work, BTW! It's great to see this sort of stuff get cleaned up! --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] qpid-proton pull request: PROTON-848-and-849: stop leaking the tra...
Github user rhs commented on the pull request: https://github.com/apache/qpid-proton/pull/20#issuecomment-95188769 Looks good to me. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] qpid-proton pull request: PROTON-853: stop erroneous attach being ...
Github user rhs commented on the pull request: https://github.com/apache/qpid-proton/pull/21#issuecomment-94755563 +1 from me Initially I thought this wouldn't account for detach properly, but looking at the code I managed to convince myself that it will. If you have time, however, it would be nice to augment/add a test of the detach scenario. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] qpid-proton pull request: PROTON-334: SASL Implementation for Prot...
Github user rhs commented on the pull request: https://github.com/apache/qpid-proton/pull/17#issuecomment-94437344 Looks good to me, assuming all the tests pass and what not, I'm +1. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] qpid-proton pull request: NO-JIRA: export.sh creates pax-based tar
Github user rhs commented on the pull request: https://github.com/apache/qpid-proton/pull/14#issuecomment-88009935 FYI I've added the updated tarballs for 0.9 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] qpid-proton pull request: NO-JIRA: export.sh creates pax-based tar
Github user rhs commented on the pull request: https://github.com/apache/qpid-proton/pull/14#issuecomment-87651630 +1 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---