Github user astitcher commented on the pull request:
https://github.com/apache/qpid-proton/pull/65#issuecomment-206491900
@hekonsek could you close this pull request now, as it is no longer for
consideration.
---
If your project is set up for it, you can reply to this email and have
Github user astitcher commented on the pull request:
https://github.com/apache/qpid-proton/pull/72#issuecomment-203974958
This looks fine modulo the inline comment.
Do you need someone else to merge it?
---
If your project is set up for it, you can reply to this email and
Github user astitcher commented on a diff in the pull request:
https://github.com/apache/qpid-proton/pull/72#discussion_r58066861
--- Diff: proton-c/bindings/javascript/CMakeLists.txt ---
@@ -192,6 +192,7 @@ target_link_libraries(qpid-proton-bitcode)
# Compile the
Github user astitcher commented on the pull request:
https://github.com/apache/qpid-proton/pull/68#issuecomment-187952497
Is there no way to tell the process that renders the markdown to html to
inspect the links and modify them as necessary (if they refer to another
markdown
Github user astitcher commented on the pull request:
https://github.com/apache/qpid-proton/pull/65#issuecomment-184349901
I think that @alanconway has a good point.
Looking at the C implementation of the url decoder it doesn't seem to
expand the % escapes for the path
GitHub user astitcher opened a pull request:
https://github.com/apache/qpid-proton/pull/63
Event tidying
Pull request for comment by @alanconway & @cliffjansen
These changes passed the tests on travis and appveyor for me, so hopefully
they work.
You can merge this
Github user astitcher commented on the pull request:
https://github.com/apache/qpid-proton/pull/57#issuecomment-171819173
Also note that the integration tests have failed - You should investigate
why that is, and comment on it if it has nothing to do with your change.
---
If your
Github user astitcher commented on the pull request:
https://github.com/apache/qpid-proton/pull/52#issuecomment-166296481
I looked at this - I can't see any problems, but I don't have enough real
expertise in the Java code to be comfortable saying it's ok and merging it
Github user astitcher commented on the pull request:
https://github.com/apache/qpid-proton/pull/47#issuecomment-123005675
I've raised [PROTON-951](https://issues.apache.org/jira/browse/PROTON-951)
and [PROTON-952](https://issues.apache.org/jira/browse/PROTON-952) - which are
re
Github user astitcher commented on the pull request:
https://github.com/apache/qpid-proton/pull/47#issuecomment-122922144
@gemmellr I'm going to try to characterise the python 3 build issue a bit
better then raise a JIRA.
---
If your project is set up for it, you can reply to
Github user astitcher commented on the pull request:
https://github.com/apache/qpid-proton/pull/47#issuecomment-122436754
The tox thing is the bit I'm shakiest about - it seems to find 3 out of
the 4 pythons (not 2.6 I think) for some reason. I experimented with a
matrix
Github user astitcher commented on the pull request:
https://github.com/apache/qpid-proton/pull/47#issuecomment-122435378
It looks to me that the appveyor failure is due to some incidental issue
with the service.
---
If your project is set up for it, you can reply to this email and
GitHub user astitcher opened a pull request:
https://github.com/apache/qpid-proton/pull/47
NO-JIRA: Change travis configuration to use container based builds
This is specifically for comment from @dnwe - I'm proposing to change the
travis build config to use containers, but
Github user astitcher commented on the pull request:
https://github.com/apache/qpid-proton/pull/45#issuecomment-120519490
I've committed these changes now, I did fix them up a bit:
Specifically:
- I changed the logic in pn_data_add() to use early return too - I must
Github user astitcher commented on the pull request:
https://github.com/apache/qpid-proton/pull/39#issuecomment-119228335
@dcristoloveanu actually given that a corrected change would be entirely
different from this change I'd actually prefer a new branch/PR to merge, which
would
Github user astitcher commented on the pull request:
https://github.com/apache/qpid-proton/pull/39#issuecomment-119227983
@dcristoloveanu - Sorry not to reply to your comments - for some reason I
got no email to tell me you had commented.
* If you change the PR I'll be
Github user astitcher commented on the pull request:
https://github.com/apache/qpid-proton/pull/35#issuecomment-109423155
There are a number of places in the API where the API has something like:
`void setXXX(const char* bytes, size_t size);` this isn't very C++ like -
in fa
Github user astitcher commented on the pull request:
https://github.com/apache/qpid-proton/pull/35#issuecomment-109421363
Another general point (I can't exactly see where to attach it in the
changes):
As well as using a class hierarchy to get callbacks, we should think
Github user astitcher commented on a diff in the pull request:
https://github.com/apache/qpid-proton/pull/35#discussion_r31844888
--- Diff: proton-c/bindings/CMakeLists.txt ---
@@ -109,6 +109,11 @@ if (EMSCRIPTEN_FOUND)
set (DEFAULT_JAVASCRIPT ON)
endif
Github user astitcher commented on a diff in the pull request:
https://github.com/apache/qpid-proton/pull/35#discussion_r31844585
--- Diff: proton-c/bindings/cpp/include/proton/cpp/Acceptor.h ---
@@ -0,0 +1,50 @@
+#ifndef PROTON_CPP_ACCEPTOR_H
+#define PROTON_CPP_ACCEPTOR_H
Github user astitcher commented on a diff in the pull request:
https://github.com/apache/qpid-proton/pull/35#discussion_r31844356
--- Diff: CMakeLists.txt ---
@@ -20,16 +20,15 @@ cmake_minimum_required (VERSION 2.6)
project (Proton C)
+# Enable C++ now for
Github user astitcher commented on a diff in the pull request:
https://github.com/apache/qpid-proton/pull/35#discussion_r31843690
--- Diff: proton-c/bindings/cpp/include/proton/cpp/Acceptor.h ---
@@ -0,0 +1,50 @@
+#ifndef PROTON_CPP_ACCEPTOR_H
+#define PROTON_CPP_ACCEPTOR_H
Github user astitcher commented on the pull request:
https://github.com/apache/qpid-proton/pull/35#issuecomment-109383948
Some overall comments (from a not yet thorough) read through:
* I really don't like using set/get in a C++ API. It's not idiomatic C++
API style
Github user astitcher commented on a diff in the pull request:
https://github.com/apache/qpid-proton/pull/35#discussion_r31834639
--- Diff: proton-c/bindings/cpp/README.md ---
@@ -0,0 +1,24 @@
+# C++ binding for proton.
+
+This is a C++ wrapper for the proton reactor
Github user astitcher commented on a diff in the pull request:
https://github.com/apache/qpid-proton/pull/35#discussion_r31834366
--- Diff: proton-c/bindings/CMakeLists.txt ---
@@ -109,6 +109,11 @@ if (EMSCRIPTEN_FOUND)
set (DEFAULT_JAVASCRIPT ON)
endif
Github user astitcher commented on the pull request:
https://github.com/apache/qpid-proton/pull/35#issuecomment-109372427
There seem to be a couple of extraneous go example files that have snuck
into this PR
---
If your project is set up for it, you can reply to this email and have
Github user astitcher commented on a diff in the pull request:
https://github.com/apache/qpid-proton/pull/35#discussion_r31833264
--- Diff: examples/cpp/example_test.py ---
@@ -0,0 +1,105 @@
+#
--- End diff --
Is this file being added by mistake? (It seems to be a
Github user astitcher commented on a diff in the pull request:
https://github.com/apache/qpid-proton/pull/35#discussion_r31832198
--- Diff: CMakeLists.txt ---
@@ -20,16 +20,15 @@ cmake_minimum_required (VERSION 2.6)
project (Proton C)
+# Enable C++ now for
Github user astitcher commented on the pull request:
https://github.com/apache/qpid-proton/pull/24#issuecomment-96902826
Oops
---
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
Github user astitcher commented on the pull request:
https://github.com/apache/qpid-proton/pull/19#issuecomment-94854481
@dcristoloveanu I've landed PROTON-334 now, so you should check that trunk
now does what you want.
---
If your project is set up for it, you can reply to
Github user astitcher commented on the pull request:
https://github.com/apache/qpid-proton/pull/17#issuecomment-93824913
@dnwe I think I copied that CMake code directly from qpid. I'll look at
doing something like you suggested - the CMake output does look clunky.
---
If
Github user astitcher commented on the pull request:
https://github.com/apache/qpid-proton/pull/17#issuecomment-91137151
See the wiki for more information and context:
https://cwiki.apache.org/confluence/x/B5cWAw
---
If your project is set up for it, you can reply to this email
GitHub user astitcher opened a pull request:
https://github.com/apache/qpid-proton/pull/17
PROTON-334: SASL Implementation for Proton-C using Cyrus SASL
This work Adds some new APIs to the transport and connection
objects to make a higher level abstraction for authentication
Github user astitcher commented on the pull request:
https://github.com/apache/qpid-proton/pull/11#issuecomment-81887757
I think you new pn_data_size() possibly has the wrong side effects - it
seems to lose the pn_data cursor position, so that merely querying the size
will be hard to
Github user astitcher commented on the pull request:
https://github.com/apache/qpid-proton/pull/11#issuecomment-78436876
> I'd love to hear some details. Are you suggesting we accumulate the
encoded size every time you
> modify the data?
Not especially, b
Github user astitcher commented on the pull request:
https://github.com/apache/qpid-proton/pull/11#issuecomment-78280044
Thinking about it a bit more, I think I particularly object to the new
public API.
What you really want at an API level here is simply a function to tell you
36 matches
Mail list logo
