Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support typescript #140

Merged
merged 5 commits into from
Jun 4, 2020
Merged

Support typescript #140

merged 5 commits into from
Jun 4, 2020

Conversation

alexduf
Copy link
Contributor

@alexduf alexduf commented May 18, 2020

This pull requests can be read commit by commit.

It:

  • updates all the dependencies of the release pipeline
  • removes unnecessary sbt configuration that scrooge already supports
  • renames shared.thrift to atomshared.thrift as there is a namespace collision with the content-entity project
  • renames from to min and to to max as from is a reserved word in thrift. This wasn't a problem as scrooge is more lax when it comes to the list of reserved words
  • add the capability to generate typescript

Before merging I'll update the content-entity version to the latest once this PR (and the following one that will add typescript) has been approved

@alexduf alexduf requested a review from a team May 18, 2020 13:40

addSbtPlugin("org.xerial.sbt" % "sbt-sonatype" % "3.7")

addSbtPlugin("com.twitter" % "scrooge-sbt-plugin" % "19.9.0")
addSbtPlugin("com.twitter" % "scrooge-sbt-plugin" % "20.4.0")
addSbtPlugin("com.gu" % "sbt-scrooge-typescript" % "1.0.1")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to try this out!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's very early still :)

@mchv
Copy link
Member

mchv commented May 19, 2020

renames from to min and to to max as from is a reserved word in thrift. This wasn't a problem as scrooge is more lax when it comes to the list of reserved words

This is why we forked thrift in the first place to remove the reserved keywords part. Apart from the official thrift compiler, those keywords should not be an issue in practice because generators should escape them in the targeting language

@alexduf
Copy link
Contributor Author

alexduf commented May 19, 2020

@mchv Yes I had a chat with @davidfurey yesterday who introduced me to French Thrift (which I believe is your creation?)

So I'll look into it today

@alexduf
Copy link
Contributor Author

alexduf commented Jun 2, 2020

This pull request now relies on the latest version of the scrooge-typescript plugin, which was completely re-written to generate the code from scala.

I've tested it locally and the generated code compiles, including the generated imports.

@alexduf
Copy link
Contributor Author

alexduf commented Jun 3, 2020

It seems to be working so far 😃

I'll merge and publish this one tomorrow morning

@alexduf alexduf merged commit ebc1892 into master Jun 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants