You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by uint <gi...@git.apache.org> on 2017/11/03 11:15:29 UTC
[GitHub] thrift pull request #1410: Common Lisp support
GitHub user uint opened a pull request:
https://github.com/apache/thrift/pull/1410
Common Lisp support
This adds Common Lisp support - library, generator, cross-tests, tutorial code, etc. Currently only SBCL is supported.
The minimal requirements are fulfilled. Cross-tests pass against most languages that I've tried.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/TurtleWarePL/thrift develop
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/thrift/pull/1410.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1410
----
commit 171e9a60784910d48006f1c1c4e08b64f81b22b4
Author: Tomek Kurcz <to...@gmail.com>
Date: 2017-09-19T07:16:43Z
Patch Thrift with de.setf.thrift
commit d4850239d69d32a11793152f9cb98b46bea55075
Author: Tomek Kurcz <to...@gmail.com>
Date: 2017-09-19T07:16:56Z
CL generator: fix and integrate it
commit a48a65de910b8769c3754bd8bdfe69ba1f0e9bc0
Author: Tomek Kurcz <to...@gmail.com>
Date: 2017-09-19T08:42:55Z
Remove non-existent packages and system dependencies
commit 1fae86f6c77bb82847607507f68b92907385b325
Author: Tomek Kurcz <to...@gmail.com>
Date: 2017-09-19T10:05:54Z
Add namespace declarations for CL in tutorial .thrift files
commit 6668f0984d936539ee13755bbdfb57f6cbe760e6
Author: Tomek Kurcz <to...@gmail.com>
Date: 2017-09-19T12:53:39Z
Fix Thrift URIs
commit f85eab281eb89d43dda5eb8cf869cbe23690b008
Author: Tomek Kurcz <to...@gmail.com>
Date: 2017-09-19T13:20:10Z
Cosmetic: Remove emacs file headers
commit a6f873e14ee33c5229e40a3101f4732d87b4caf7
Author: Tomek Kurcz <to...@gmail.com>
Date: 2017-09-20T10:35:06Z
Fix the handling of Thrift types
The decoder should expect i32 when the field type is enum
Rename i08 to i8 according to Thrift's expectations
commit b6c44618c3776e9d5ce80e9276b9e176b7fb96f9
Author: Tomek Kurcz <to...@gmail.com>
Date: 2017-09-21T06:39:10Z
Defined services should just be exported to the current *package*
commit dddd6393e7d75cec8822d6f57d4cec14a6563528
Author: Tomek Kurcz <to...@gmail.com>
Date: 2017-09-21T07:38:19Z
Bugfix: wrong order of arguments in a function call
commit 8cafef222a8a8f532627a7f31c5f5a7568954283
Author: Tomek Kurcz <to...@gmail.com>
Date: 2017-09-22T09:17:31Z
Ensure users can use :common-lisp symbols when implementing services
commit c413c64ff7699151916a01638c1f02d37b473acc
Author: Tomek Kurcz <to...@gmail.com>
Date: 2017-09-22T11:29:55Z
CL generator: Generate ASDF systems for Thrift programs
Also adds the CLI option not to generate .asd files, but the default is to
generate them.
commit cf85ca14704c51aeafd926bc4cc14f6fdbdd3117
Author: Tomek Kurcz <to...@gmail.com>
Date: 2017-09-22T11:50:10Z
Don't expect server implementation to exist when loading gen'd code
We probably expect those to be declared later
commit 11bdf23b208dad9c2a95d0e1350782c75b4b1bb6
Author: Tomek Kurcz <to...@gmail.com>
Date: 2017-09-25T09:50:26Z
CL generator: Put generated "programs" in separate directories
commit 19ff832d21599618c389fba44963f519c4637712
Author: Tomek Kurcz <to...@gmail.com>
Date: 2017-09-25T11:20:14Z
CL generator: copy the options string to comments in generated files
commit cfdbc1e2ad1bd1d8a66888a0db17879c9cae0562
Author: Tomek Kurcz <to...@gmail.com>
Date: 2017-09-25T11:33:02Z
CL generator: Fix the remaining warnings
commit 340486f190a9cc66555dbdaac6c6ee463bfb615a
Author: Tomek Kurcz <to...@gmail.com>
Date: 2017-09-25T12:08:39Z
Replace the float conversion code with `ieee-floats` from quicklisp
commit c990ad183817e1199aa222afa38996c7dd405e0e
Author: Tomek Kurcz <to...@gmail.com>
Date: 2017-09-26T08:25:46Z
CL generator: Add the option to change the ASDF system prefix
Also: Cosmetic: add a newline after a generated (def-service)
commit fb63de742ccf20d4324cdf20ded1aeca7c81dde4
Author: Tomek Kurcz <to...@gmail.com>
Date: 2017-09-27T08:32:52Z
Fix the load order of thrift-test components
commit 2e0fb30a62f1881fc6cb3946cff20e5c46096f4a
Author: Tomek Kurcz <to...@gmail.com>
Date: 2017-09-27T08:33:17Z
Export vector-stream for use in thrift-test
commit 80494401afcc52bbb5e6d40bcd374986078d195c
Author: Tomek Kurcz <to...@gmail.com>
Date: 2017-09-27T08:38:43Z
Exclude definition-operators.lisp from compilation for now
commit f847cf503dc2cba7f3efb34bef04793d0a068a3c
Author: Tomek Kurcz <to...@gmail.com>
Date: 2017-09-27T08:53:34Z
Cosmetic: Typos and reindentation
commit 61c60b4d3b3c76ce46868b4e9c2a949cf5c4fc4c
Author: Tomek Kurcz <to...@gmail.com>
Date: 2017-09-27T09:01:14Z
Fix vector-protocol.write-sequence
commit 4910c4285df08de009d0fbaa42118b1775f737c3
Author: Tomek Kurcz <to...@gmail.com>
Date: 2017-09-27T09:08:43Z
Fix calls to unexported thrift.implementation functions
commit 314310dadbaa3d3cd8327942f71af38c9ce2accf
Author: Tomek Kurcz <to...@gmail.com>
Date: 2017-09-28T09:56:03Z
Make sure make-test-server.lisp is loaded with relative path
commit 0c279b847f3c70647857c385d9ffc4f760b014f3
Author: Tomek Kurcz <to...@gmail.com>
Date: 2017-09-28T10:06:29Z
Make make-test-server.lisp load the Thrift library
commit 1af3b7632f07dda6503bb9ad33b9f3e2ee614602
Author: Tomek Kurcz <to...@gmail.com>
Date: 2017-09-29T08:27:17Z
Hook up Common Lisp cross tests to Thrift's build system
commit 385da79e19f04ca190942e1a84c058c9875326c7
Author: Tomek Kurcz <to...@gmail.com>
Date: 2017-09-29T12:05:22Z
CL cross: Implement the CL server for cross tests
Add the CL cross-test server to tests.json
commit 8573015f4362f49d783a5a8a00bb5c07f3da1a72
Author: Tomek Kurcz <to...@gmail.com>
Date: 2017-09-30T10:19:14Z
Bugfix: field-type code 3 should be translated to i8, not byte
commit ec8c7a8e4f85784df513cb6b4eef502dd1227e24
Author: Tomek Kurcz <to...@gmail.com>
Date: 2017-10-02T07:47:10Z
Bugfix: ensure the generator generates 'binary' where apt
commit c439c303c0b5b2757bbca96c57ddf43fa6f2913e
Author: Tomek Kurcz <to...@gmail.com>
Date: 2017-10-02T09:16:13Z
Bugfix: i32 should be expected/written for enum fields
----
---
[GitHub] thrift pull request #1410: THRIFT-82: Add Common Lisp support
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1410#discussion_r149446200
--- Diff: lib/cl/ensure-externals.sh ---
@@ -0,0 +1,12 @@
+#!/bin/sh
+
+set -e
+
+if [ ! -d "externals/" ] ; then
--- End diff --
What happens if externals are partially populated due to a crash? Can one run the same quicklisp command idempotently? If so, I would remove this if statement and just always run quicklisp to make sure externals are correct. You could perhaps make the curl command smarter to only update the file if it has changed - I suspect curl has an option for that, but I don't know...
---
[GitHub] thrift pull request #1410: THRIFT-82: Add Common Lisp support
Posted by uint <gi...@git.apache.org>.
Github user uint closed the pull request at:
https://github.com/apache/thrift/pull/1410
---
[GitHub] thrift pull request #1410: THRIFT-82: Add Common Lisp support
Posted by dkochmanski <gi...@git.apache.org>.
Github user dkochmanski commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1410#discussion_r149489770
--- Diff: lib/cl/server.lisp ---
@@ -0,0 +1,230 @@
+(in-package #:org.apache.thrift.implementation)
--- End diff --
it is a common practice in Common Lisp to have a package dedicated for usage by a programmer (which exports the protocol symbols - library API that is) and having no other symbols of its own and an implementation package which may have other internal symbols and interfaces. This allows avoiding for instance symbol name conflicts (for instance list vs list).
---
[GitHub] thrift issue #1410: THRIFT-82: Add Common Lisp support
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1410
Without a linux build, won't be able to do any merges...
---
[GitHub] thrift issue #1410: THRIFT-82: Add Common Lisp support
Posted by jfarrell <gi...@git.apache.org>.
Github user jfarrell commented on the issue:
https://github.com/apache/thrift/pull/1410
@dkochmanski Travis support responded saying that no one from TurtleWarePL has logged into https://travis-ci.org/ ever and this is the reason this PR is not getting run within travis.
From Travis support:
> do you think one of their members could try to log into Travis CI at least once at https://travis-ci.org/ and try closing and reopen one of the rejected Pull Requests?
---
[GitHub] thrift issue #1410: THRIFT-82: Add Common Lisp support
Posted by Jens-G <gi...@git.apache.org>.
Github user Jens-G commented on the issue:
https://github.com/apache/thrift/pull/1410
@jfarrell: What's your opinion re the (c) things above?
---
[GitHub] thrift pull request #1410: Common Lisp support
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1410#discussion_r148790161
--- Diff: lib/cl/externals/bundle.lisp ---
@@ -0,0 +1,161 @@
+(cl:in-package #:cl-user)
--- End diff --
Missing license statement.
---
[GitHub] thrift pull request #1410: Common Lisp support
Posted by dkochmanski <gi...@git.apache.org>.
Github user dkochmanski commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1410#discussion_r148796969
--- Diff: lib/cl/externals/bundle-info.sexp ---
@@ -0,0 +1,14 @@
+(:CREATION-TIME #A((20) BASE-CHAR . "2017-10-31T11:49:23Z") :REQUESTED-SYSTEMS
--- End diff --
all things put in `externals` are checked in, so whole directory will be removed (this file included).
---
[GitHub] thrift issue #1410: Common Lisp support
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1410
@jfarrell for some reason there is no recorded Travis CI build for this pull request. When the author squashes and resubmits hopefully this will kick off a proper build?
---
[GitHub] thrift issue #1410: THRIFT-82: Add Common Lisp support
Posted by jfarrell <gi...@git.apache.org>.
Github user jfarrell commented on the issue:
https://github.com/apache/thrift/pull/1410
Travis requests show an error with "Abuse detected". I've emailed Travis support asking them to look into the issue
---
[GitHub] thrift issue #1410: Common Lisp support
Posted by dkochmanski <gi...@git.apache.org>.
Github user dkochmanski commented on the issue:
https://github.com/apache/thrift/pull/1410
Thank you for taking your time for the review
---
[GitHub] thrift pull request #1410: THRIFT-82: Add Common Lisp support
Posted by dkochmanski <gi...@git.apache.org>.
Github user dkochmanski commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1410#discussion_r149592614
--- Diff: lib/cl/framed-transport.lisp ---
@@ -0,0 +1,136 @@
+(in-package #:org.apache.thrift.implementation)
+
+;;;; Copyright 2017 Rigetti Computing <rigetti.com>
--- End diff --
Right, thank you for pointing me to the document. Last commit removes all copyrights added by us. Note though, that files in lib/cl/ are based on work by James Anderson (and have copyrights as well) and I don't think I can remove them. Should I contact him and ask whenever we can remove his copyright stings too?
---
[GitHub] thrift issue #1410: THRIFT-82: Add Common Lisp support
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1410
Let's follow @jfarrell 's lead. Whatever he says, he's managing the travis interaction.
---
[GitHub] thrift pull request #1410: Common Lisp support
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1410#discussion_r148791102
--- Diff: lib/cl/framed-transport.lisp ---
@@ -0,0 +1,136 @@
+(in-package #:org.apache.thrift.implementation)
+
+;;;; Copyright 2017 Rigetti Computing <rigetti.com>
--- End diff --
@jfarrell do we need to check with them to make sure this is okay to submit?
---
[GitHub] thrift pull request #1410: Common Lisp support
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1410#discussion_r148790890
--- Diff: lib/cl/externals/software/usocket-0.7.0.1/vendor/OpenTransportUDP.lisp ---
@@ -0,0 +1,146 @@
+;;;-*-Mode: LISP; Package: CCL -*-
--- End diff --
As with the other libraries, this should not be checked in. I have skipped over all of the externals/ due to this.
---
[GitHub] thrift issue #1410: Common Lisp support
Posted by uint <gi...@git.apache.org>.
Github user uint commented on the issue:
https://github.com/apache/thrift/pull/1410
So about removing the bundled dependencies (`lib/cl/externals`).
There is a nifty package manager for CL (Quicklisp) that works a bit like, say, Cargo for Rust. It's commonly used, but the problem is it isn't available in `PATH` or anything - it's installed in the user's dir normally. There's no good way to detect its presence while building Thrift.
What if we included the Quicklisp installer (literally a single `.lisp` file), installed it locally during the build, and then used it to pull in other dependencies? Would that be fine?
---
[GitHub] thrift issue #1410: THRIFT-82: Add Common Lisp support
Posted by dkochmanski <gi...@git.apache.org>.
Github user dkochmanski commented on the issue:
https://github.com/apache/thrift/pull/1410
Hello, I've send you an email a few hours ago with a question, if we can remove your "copyright" headers from the code found in de.setf.thrift, because that is what is required by Thrift team (I have included details in the email). Plese see: https://github.com/apache/thrift/pull/1410#discussion_r149655532 .
---
[GitHub] thrift pull request #1410: THRIFT-82: Add Common Lisp support
Posted by dkochmanski <gi...@git.apache.org>.
Github user dkochmanski commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1410#discussion_r149660082
--- Diff: lib/cl/framed-transport.lisp ---
@@ -0,0 +1,136 @@
+(in-package #:org.apache.thrift.implementation)
+
+;;;; Copyright 2017 Rigetti Computing <rigetti.com>
--- End diff --
I've send an email to Mr. Anderson – will let you know when / if I receive a reply. No, it is not possible, our work takes his repository[1] as base for further adjustments to meet the contribution requirements (missing protocols etc).
[1] https://github.com/lisp/de.setf.thrift
---
[GitHub] thrift pull request #1410: Common Lisp support
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1410#discussion_r148790488
--- Diff: lib/cl/externals/software/alexandria-20170830-git/.boring ---
@@ -0,0 +1,13 @@
+# Boring file regexps:
--- End diff --
If you checked in alexandria, it should not be part of the submission. Instead, the build process should install it on demand in the same manner that the nodejs implementation calls npm to install packages during the build.
---
[GitHub] thrift issue #1410: THRIFT-82: Add Common Lisp support
Posted by dkochmanski <gi...@git.apache.org>.
Github user dkochmanski commented on the issue:
https://github.com/apache/thrift/pull/1410
closing, will reopen with squashed commits.
---
[GitHub] thrift issue #1410: Common Lisp support
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1410
Some initial thoughts, before I review all of the files individually:
1. Please squash.
2. Please add [THRIFT-82] at the beginning of the commit description and the pull request description.
3. Please review https://thrift.apache.org/docs/HowToContribute.
I resurrected https://issues.apache.org/jira/browse/THRIFT-82 upon seeing this. I'll start going through the files.
---
[GitHub] thrift pull request #1410: THRIFT-82: Add Common Lisp support
Posted by jfarrell <gi...@git.apache.org>.
Github user jfarrell commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1410#discussion_r149508723
--- Diff: lib/cl/framed-transport.lisp ---
@@ -0,0 +1,136 @@
+(in-package #:org.apache.thrift.implementation)
+
+;;;; Copyright 2017 Rigetti Computing <rigetti.com>
--- End diff --
@dkochmanski thank you for working on this and contributing it back the the Apache Thrift. The Apache license header on this should not contain a copyright to Rigetti Computing, details available at https://www.apache.org/legal/src-headers.html#headers part 2
---
[GitHub] thrift pull request #1410: THRIFT-82: Add Common Lisp support
Posted by Jens-G <gi...@git.apache.org>.
Github user Jens-G commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1410#discussion_r149504639
--- Diff: lib/cl/framed-transport.lisp ---
@@ -0,0 +1,136 @@
+(in-package #:org.apache.thrift.implementation)
+
+;;;; Copyright 2017 Rigetti Computing <rigetti.com>
--- End diff --
Well, the term "sponsored" is a bit broad to derive that particular conclusion from it. So I asked for further clarification.
---
[GitHub] thrift issue #1410: THRIFT-82: Add Common Lisp support
Posted by lisp <gi...@git.apache.org>.
Github user lisp commented on the issue:
https://github.com/apache/thrift/pull/1410
good evening; this popped up in my mail due to mr kochmanski's reference.
what are you waiting for from me?
---
[GitHub] thrift issue #1410: THRIFT-82: Add Common Lisp support
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1410
I think it would be better to make a shell script that downloads quicklisp and runs it to install the dependencies, and make the local build in lib/cl depend on successful execution of the shell script. This makes it similar to the nodejs build which runs npm to do something similar, however the only difference is that npm is installed as part of the docker build.
.
---
[GitHub] thrift pull request #1410: THRIFT-82: Add Common Lisp support
Posted by jfarrell <gi...@git.apache.org>.
Github user jfarrell commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1410#discussion_r149655532
--- Diff: lib/cl/framed-transport.lisp ---
@@ -0,0 +1,136 @@
+(in-package #:org.apache.thrift.implementation)
+
+;;;; Copyright 2017 Rigetti Computing <rigetti.com>
--- End diff --
That would be great. is it possible fetch his work as a third party downloadable dependency or was it used as the basis for work you did on top of it?
---
[GitHub] thrift issue #1410: THRIFT-82: Add Common Lisp support
Posted by dkochmanski <gi...@git.apache.org>.
Github user dkochmanski commented on the issue:
https://github.com/apache/thrift/pull/1410
Hey, removed all externals and added `ensure-externals.sh` script. Also squashed all commits.
---
[GitHub] thrift pull request #1410: Common Lisp support
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1410#discussion_r148790023
--- Diff: lib/cl/externals/bundle-info.sexp ---
@@ -0,0 +1,14 @@
+(:CREATION-TIME #A((20) BASE-CHAR . "2017-10-31T11:49:23Z") :REQUESTED-SYSTEMS
--- End diff --
Is this a file that was supposed to be checked in? It looks like it is machine-specific.
---
[GitHub] thrift issue #1410: THRIFT-82: Add Common Lisp support
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1410
Why don't we close this PR and open a new one free of any copyright issues or inclusion of third party code. That should resolve the issue?
---
[GitHub] thrift pull request #1410: THRIFT-82: Add Common Lisp support
Posted by dkochmanski <gi...@git.apache.org>.
Github user dkochmanski commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1410#discussion_r149491753
--- Diff: lib/cl/ensure-externals.sh ---
@@ -0,0 +1,12 @@
+#!/bin/sh
+
+set -e
+
+if [ ! -d "externals/" ] ; then
--- End diff --
it is possible. Fixed in the pushed commit (some extra eval forms are explained in the commit message.
---
[GitHub] thrift pull request #1410: Common Lisp support
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1410#discussion_r148789088
--- Diff: compiler/cpp/src/thrift/generate/t_cl_generator.cc ---
@@ -0,0 +1,544 @@
+// Copyright (c) 2008- Patrick Collison <pa...@collison.ie>
--- End diff --
This needs the standard Apache Software License statement applied.
---
[GitHub] thrift pull request #1410: Common Lisp support
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1410#discussion_r148791220
--- Diff: lib/cl/binary-protocol.lisp ---
@@ -0,0 +1,255 @@
+(in-package #:org.apache.thrift.implementation)
+
+;;;; This file defines the concrete `binary-protocol` layer for the `org.apache.thrift` library.
+;;;;
+;;;; copyright 2010 [james anderson](james.anderson@setf.de)
--- End diff --
@jfarrell do we need to check with them to make sure this is okay to submit?
---
[GitHub] thrift issue #1410: THRIFT-82: Add Common Lisp support
Posted by dkochmanski <gi...@git.apache.org>.
Github user dkochmanski commented on the issue:
https://github.com/apache/thrift/pull/1410
@jfarrell logged into travis a moment ago for the first time.
@jeking3 I'm waiting for a response from @lisp if we can remove his copyright strings (we've removed ours). Should I close the PR and issue a new one before I receive a reply?
---
[GitHub] thrift pull request #1410: THRIFT-82: Add Common Lisp support
Posted by dkochmanski <gi...@git.apache.org>.
Github user dkochmanski commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1410#discussion_r149489004
--- Diff: lib/cl/framed-transport.lisp ---
@@ -0,0 +1,136 @@
+(in-package #:org.apache.thrift.implementation)
+
+;;;; Copyright 2017 Rigetti Computing <rigetti.com>
--- End diff --
Rigetti hired us to implement (and possibly merge) CL support to Thrift. All software under this delivery has to have license required by the project we are contributing to (that is Thrift). Not sure what is strange about that.
---
[GitHub] thrift pull request #1410: THRIFT-82: Add Common Lisp support
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1410#discussion_r149446453
--- Diff: lib/cl/server.lisp ---
@@ -0,0 +1,230 @@
+(in-package #:org.apache.thrift.implementation)
--- End diff --
Does one need the ".implementation" in the namespace?
---
[GitHub] thrift pull request #1410: Common Lisp support
Posted by dkochmanski <gi...@git.apache.org>.
Github user dkochmanski commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1410#discussion_r148797236
--- Diff: lib/cl/framed-transport.lisp ---
@@ -0,0 +1,136 @@
+(in-package #:org.apache.thrift.implementation)
+
+;;;; Copyright 2017 Rigetti Computing <rigetti.com>
--- End diff --
Our work on Thrift is sponsored by Rigetti, so there is no need for that.
---
[GitHub] thrift issue #1410: THRIFT-82: Add Common Lisp support
Posted by dkochmanski <gi...@git.apache.org>.
Github user dkochmanski commented on the issue:
https://github.com/apache/thrift/pull/1410
see https://github.com/apache/thrift/pull/1412
---
[GitHub] thrift pull request #1410: THRIFT-82: Add Common Lisp support
Posted by Jens-G <gi...@git.apache.org>.
Github user Jens-G commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1410#discussion_r149475953
--- Diff: lib/cl/framed-transport.lisp ---
@@ -0,0 +1,136 @@
+(in-package #:org.apache.thrift.implementation)
+
+;;;; Copyright 2017 Rigetti Computing <rigetti.com>
--- End diff --
> Our work on Thrift is sponsored by Rigetti, so there is no need for that.
That sounds ... strange.
---
[GitHub] thrift pull request #1410: Common Lisp support
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1410#discussion_r148790668
--- Diff: lib/cl/externals/software/bordeaux-threads-v0.8.5/.travis.yml ---
@@ -0,0 +1,44 @@
+language: lisp
--- End diff --
If you checked in bordeaux-threads, it should not be part of the submission. Instead, the build process should install it on demand in the same manner that the nodejs implementation calls npm to install packages during the build.
---