[GitHub] thrift pull request: THRIFT-2640: Compact Protocol in Cocoa

2015-04-27 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/thrift/pull/442 --- 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 enab

[GitHub] thrift pull request: THRIFT-2640: Compact Protocol in Cocoa

2015-04-27 Thread jeremy-w
Github user jeremy-w commented on the pull request: https://github.com/apache/thrift/pull/442#issuecomment-96656933 :+1: Sounds 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 thi

[GitHub] thrift pull request: THRIFT-2640: Compact Protocol in Cocoa

2015-04-26 Thread creker
GitHub user creker reopened a pull request: https://github.com/apache/thrift/pull/442 THRIFT-2640: Compact Protocol in Cocoa You can merge this pull request into a Git repository by running: $ git pull https://github.com/creker/thrift THRIFT-2640 Alternatively you can review

[GitHub] thrift pull request: THRIFT-2640: Compact Protocol in Cocoa

2015-04-26 Thread creker
Github user creker closed the pull request at: https://github.com/apache/thrift/pull/442 --- 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 enab

[GitHub] thrift pull request: THRIFT-2640: Compact Protocol in Cocoa

2015-04-21 Thread creker
Github user creker commented on the pull request: https://github.com/apache/thrift/pull/442#issuecomment-94731417 I think we should start by fixing protocols because all other code is mostly depending on them. After that it should be easy to fix type issues. --- If your project is se

[GitHub] thrift pull request: THRIFT-2640: Compact Protocol in Cocoa

2015-04-20 Thread jeremy-w
Github user jeremy-w commented on the pull request: https://github.com/apache/thrift/pull/442#issuecomment-94577409 Where possible without changing the protocol bits that are pinned by codegen, it would be good to address integer width/signedness change issues or add the appropriate u

[GitHub] thrift pull request: THRIFT-2640: Compact Protocol in Cocoa

2015-04-20 Thread jeremy-w
Github user jeremy-w commented on the pull request: https://github.com/apache/thrift/pull/442#issuecomment-94576314 In that case, the remaining warnings I'm seeing are mostly implicit sign conversions and implicit precision loss (integer width narrowing), with a smattering of "ARC wou

[GitHub] thrift pull request: THRIFT-2640: Compact Protocol in Cocoa

2015-04-20 Thread Jens-G
Github user Jens-G commented on the pull request: https://github.com/apache/thrift/pull/442#issuecomment-94569429 > > I would like to hear someone elses opinion before I'm going to merge > A squash-merge is fine by me. Gtreat, but that wasn't really the question ;-) What I

[GitHub] thrift pull request: THRIFT-2640: Compact Protocol in Cocoa

2015-04-20 Thread jeremy-w
Github user jeremy-w commented on the pull request: https://github.com/apache/thrift/pull/442#issuecomment-94568290 A squash-merge is fine by me. I am happy @creker made many small commits as part of the pull request, because it is much easier to review. After squash-merging,

AW: [GitHub] thrift pull request: THRIFT-2640: Compact Protocol in Cocoa

2015-04-19 Thread Jens Geyer
HRIFT-2640: Compact Protocol in Cocoa Github user creker commented on the pull request: https://github.com/apache/thrift/pull/442#issuecomment-94223036 Do you need me to squash the commits? --- If your project is set up for it, you can reply to this email and have your reply appear on G

[GitHub] thrift pull request: THRIFT-2640: Compact Protocol in Cocoa

2015-04-18 Thread creker
Github user creker commented on the pull request: https://github.com/apache/thrift/pull/442#issuecomment-94223036 Do you need me to squash the commits? --- 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

[GitHub] thrift pull request: THRIFT-2640: Compact Protocol in Cocoa

2015-04-17 Thread creker
Github user creker commented on the pull request: https://github.com/apache/thrift/pull/442#issuecomment-94009192 Thanks for the comments! I'll try to fix as much as I can. The code is straight port from C# with C++ bits here and there so I expected it woudn't be merged strai

[GitHub] thrift pull request: THRIFT-2640: Compact Protocol in Cocoa

2015-04-17 Thread jeremy-w
Github user jeremy-w commented on the pull request: https://github.com/apache/thrift/pull/442#issuecomment-94005616 If you would be so kind as to fix the `ttypeToCompactType` bit, I think this would be fine to merge. The other fixes can be done later. --- If your project is set up fo

[GitHub] thrift pull request: THRIFT-2640: Compact Protocol in Cocoa

2015-04-17 Thread jeremy-w
Github user jeremy-w commented on a diff in the pull request: https://github.com/apache/thrift/pull/442#discussion_r28602218 --- Diff: lib/cocoa/src/protocol/TCompactProtocol.m --- @@ -0,0 +1,706 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * o

[GitHub] thrift pull request: THRIFT-2640: Compact Protocol in Cocoa

2015-04-17 Thread jeremy-w
Github user jeremy-w commented on a diff in the pull request: https://github.com/apache/thrift/pull/442#discussion_r28601980 --- Diff: lib/cocoa/src/protocol/TCompactProtocol.m --- @@ -0,0 +1,706 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * o

[GitHub] thrift pull request: THRIFT-2640: Compact Protocol in Cocoa

2015-04-17 Thread jeremy-w
Github user jeremy-w commented on a diff in the pull request: https://github.com/apache/thrift/pull/442#discussion_r28601814 --- Diff: lib/cocoa/src/protocol/TCompactProtocol.m --- @@ -0,0 +1,706 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * o

[GitHub] thrift pull request: THRIFT-2640: Compact Protocol in Cocoa

2015-04-17 Thread jeremy-w
Github user jeremy-w commented on a diff in the pull request: https://github.com/apache/thrift/pull/442#discussion_r28601636 --- Diff: lib/cocoa/src/protocol/TCompactProtocol.m --- @@ -0,0 +1,706 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * o

[GitHub] thrift pull request: THRIFT-2640: Compact Protocol in Cocoa

2015-04-17 Thread jeremy-w
Github user jeremy-w commented on a diff in the pull request: https://github.com/apache/thrift/pull/442#discussion_r28601691 --- Diff: lib/cocoa/src/protocol/TCompactProtocol.m --- @@ -0,0 +1,706 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * o

[GitHub] thrift pull request: THRIFT-2640: Compact Protocol in Cocoa

2015-04-17 Thread jeremy-w
Github user jeremy-w commented on a diff in the pull request: https://github.com/apache/thrift/pull/442#discussion_r28600942 --- Diff: lib/cocoa/src/protocol/TCompactProtocol.m --- @@ -0,0 +1,706 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * o

[GitHub] thrift pull request: THRIFT-2640: Compact Protocol in Cocoa

2015-04-17 Thread jeremy-w
Github user jeremy-w commented on a diff in the pull request: https://github.com/apache/thrift/pull/442#discussion_r28600843 --- Diff: lib/cocoa/src/protocol/TCompactProtocol.m --- @@ -0,0 +1,706 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * o

[GitHub] thrift pull request: THRIFT-2640: Compact Protocol in Cocoa

2015-04-17 Thread jeremy-w
Github user jeremy-w commented on a diff in the pull request: https://github.com/apache/thrift/pull/442#discussion_r28600626 --- Diff: lib/cocoa/src/protocol/TCompactProtocol.m --- @@ -0,0 +1,706 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * o

[GitHub] thrift pull request: THRIFT-2640: Compact Protocol in Cocoa

2015-04-17 Thread jeremy-w
Github user jeremy-w commented on the pull request: https://github.com/apache/thrift/pull/442#issuecomment-94001001 It would be nice if this built cleanly with the following warning config: `OTHER_CFLAGS = -Wall -Wextra -Weverything -Wno-objc-missing-property-synthesis -Wno-unused-par

[GitHub] thrift pull request: THRIFT-2640: Compact Protocol in Cocoa

2015-04-17 Thread jeremy-w
Github user jeremy-w commented on a diff in the pull request: https://github.com/apache/thrift/pull/442#discussion_r28598871 --- Diff: lib/cocoa/src/protocol/TCompactProtocol.h --- @@ -0,0 +1,36 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or

[GitHub] thrift pull request: THRIFT-2640: Compact Protocol in Cocoa

2015-04-15 Thread creker
GitHub user creker opened a pull request: https://github.com/apache/thrift/pull/442 THRIFT-2640: Compact Protocol in Cocoa You can merge this pull request into a Git repository by running: $ git pull https://github.com/creker/thrift THRIFT-2640 Alternatively you can review an