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

Parameterize package name #17

Merged
merged 11 commits into from
May 17, 2018
Merged

Conversation

jiminhsieh
Copy link
Contributor

No description provided.

Copy link
Contributor

@raboof raboof left a comment

Choose a reason for hiding this comment

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

Many nice cleanups all around, thanks! Some comments here and there.

@@ -26,16 +26,6 @@ Once inside the project folder, run the application with:
sbt run
```

This template also provides build descriptors for maven and gradle. You can use any of the following commands to run
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm good catch

@@ -1,2 +0,0 @@
name=akka-quickstart-scala
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we sure this is unused? The text is suspiciously the same as on https://developer.lightbend.com/start/?group=akka

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are we sure this is unused?

I am not so sure, but I didn't find the same file at https://github.com/akka/akka-http-quickstart-scala.g8 and https://github.com/akka/akka-http-quickstart-java.g8. I also found this default.properties and src/main/g8/default.properties shared almost the same property values. I think I can restore since there are considerations.

BTW, I noticed the link icon of github of https://developer.lightbend.com/start/?group=akka&project=akka-http-quickstart-java and https://developer.lightbend.com/start/?group=akka&project=akka-http-quickstart-scala are both linking to 10.0.x branch.


lazy val akkaVersion = "2.5.12"
lazy val akkaVersion = "$akka_version$"
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@@ -50,7 +50,7 @@ class Printer extends Actor with ActorLogging {

def receive = {
case Greeting(greeting) =>
log.info(s"Greeting received (from ${sender()}): $greeting")
log.info("Greeting received (from " + sender() + "): " + greeting)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a bit of a shame, wouldn't there be some other way to get rid of the Fix mismatched input '$' expecting LPAREN` warning?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know we can use backslash \ to handle this,

log.info(s"Greeting received (from \${sender()}): \$greeting")

but paradox didn't handle this. It will appear those escape characters at Lightbend Tech Hub. That's why I chose this way. If there is another way to handle this, please let me know. Thanks! :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, so it's an interaction between paradox and g8: since we removed verbatim = *.scala (to make the variable substitution for the package name work) it now also tries to interpret the $ signs in the rest of the code, but paradox isn't prepared for that. This is fine for now then.

Copy link
Contributor

Choose a reason for hiding this comment

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

(added #20 so we can revisit this later)

# Conflicts:
#	src/main/g8/src/test/scala/$package$/AkkaQuickstartSpec.scala
@raboof raboof merged commit 544418d into akka:2.5.x May 17, 2018
@raboof
Copy link
Contributor

raboof commented May 17, 2018

Thanks again, great improvement!

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.

2 participants