[GitHub] qpid-proton pull request: PROTON-881: Make connect a non-blocking ...

2015-07-05 Thread rhs
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

2015-05-06 Thread rhs
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

2015-05-06 Thread rhs
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

2015-05-06 Thread rhs
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

2015-05-06 Thread rhs
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

2015-04-28 Thread rhs
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

2015-04-28 Thread rhs
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

2015-04-27 Thread rhs
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

2015-04-24 Thread rhs
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...

2015-04-22 Thread rhs
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 ...

2015-04-21 Thread rhs
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...

2015-04-20 Thread rhs
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

2015-03-31 Thread rhs
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

2015-03-30 Thread rhs
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.
---