Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Add support for Python; refactor support for Ruby #1

Merged
8 commits merged into from Mar 14, 2011

Conversation

Projects
None yet
2 participants
Contributor

olix0r commented Mar 12, 2011

This change refactors CompileThriftRuby out of CompileThriftJava, and also introduces CompileThriftPython and CompileThriftPythonTwisted (all of which extend the common base CompileThrift).

Oliver Gould added some commits Mar 12, 2011

Oliver Gould Refactor Ruby out of CompileThriftJava.
Add Python [+Twisted] support.
c766f84
Oliver Gould Update README with changes eb3d3c5
Oliver Gould typo 1706a3d
Oliver Gould Fix typos and compile errors f877cbc

@robey robey and 1 other commented on an outdated diff Mar 12, 2011

src/main/scala/com/twitter/sbt/CompileThriftPython.scala
@@ -0,0 +1,25 @@
+package com.twitter.sbt
+
+import _root_.sbt._
+
+
+trait CompileThriftPython extends CompileThrift {
+ lazy val compileThriftPython =
+ compileThriftAction("py:new_style") describedAs("Compile thrift into python")
+
+ lazy val autoCompileThriftPython = task {
+ if (autoCompileThriftEnabled)
@olix0r

olix0r Mar 12, 2011

Contributor

this should be without the newline?

@robey

robey Mar 14, 2011

just that it should be like:

if (foo) {
bar
} else {
baz
}

@robey

robey Mar 14, 2011

ugh, github messed it up, but the idea is that the scala code style says use braces if the "if" takes up more than one line.

@robey robey and 1 other commented on an outdated diff Mar 12, 2011

src/main/scala/com/twitter/sbt/CompileThriftPython.scala
@@ -0,0 +1,25 @@
+package com.twitter.sbt
+
+import _root_.sbt._
+
+
+trait CompileThriftPython extends CompileThrift {
+ lazy val compileThriftPython =
+ compileThriftAction("py:new_style") describedAs("Compile thrift into python")
+
+ lazy val autoCompileThriftPython = task {
+ if (autoCompileThriftEnabled)
+ compileThriftPython.run
+ else {
+ log.info(name+": not auto-compiling thrift-python; you may need to run compile-thrift-python manually")
@robey

robey Mar 12, 2011

code style (name + ")

@olix0r

olix0r Mar 12, 2011

Contributor

this was copy/pasted, but the dude abides

@robey

robey Mar 14, 2011

yeah, sorry, i realized that it was a paste halfway thru the review, but it would still be good to fix them while they're being messed with.

@robey robey and 1 other commented on an outdated diff Mar 12, 2011

src/main/scala/com/twitter/sbt/CompileThriftRuby.scala
@@ -0,0 +1,18 @@
+package com.twitter.sbt
+
+import _root_.sbt._
+
+
+trait CompileThriftRuby extends CompileThrift {
+ lazy val compileThriftRuby = compileThriftAction("rb") describedAs("Compile thrift into ruby")
+
+ lazy val autoCompileThriftRuby = task {
+ if (autoCompileThriftEnabled) compileThriftRuby.run
@olix0r

olix0r Mar 12, 2011

Contributor

eh? this was copy/pasted from CompileThirftJava... should this have brackets?

@robey

robey Mar 14, 2011

just another if/else block style thing.

@robey robey commented on the diff Mar 12, 2011

src/main/scala/com/twitter/sbt/GeneratedSources.scala
override def mainSourceRoots = super.mainSourceRoots +++ (outputPath / generatedJavaDirectoryName ##)
- lazy val cleanGenerated = (cleanTask(generatedJavaPath) && cleanTask(generatedRubyPath)) describedAs
- "Clean generated source folders"
+ lazy val cleanGenerated = (
@olix0r

olix0r Mar 12, 2011

Contributor

should be out-dented?

i.e.:
val foo = (
expr && expr
) decorate()

?

@robey

robey Mar 14, 2011

yeah, to keep the blocks aligned.

robey commented Mar 12, 2011

+1 after code style... tho it occurs to me that it's confusing to have ruby be automatic with java now. might as well make the all be explicit.

Contributor

olix0r commented Mar 12, 2011

Ruby is not automatically generated with Java? only with Scala (which is necessary afaiu)

Oliver Gould added some commits Mar 13, 2011

Oliver Gould Style scrub e818049
Oliver Gould Make twisted-generation a feature of python-generation.
(A flag rather than a separate class)
bb71f00
Oliver Gould avoid possible namespace collision fbdf07b

robey commented Mar 14, 2011

i guess just leave that for now. it just occurred to me that it's a little confusing that ruby bindings are auto-generated for some cases, but that's not true of any other language.

This issue was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment